GUI code problem

[baf06]

I have this GUI code that is supposed to form a shopping list were the users can order some items and the total price of the items shown on every order taken, i have done all the work, but faced one problem.

private ArrayList<ItemOrder> cart;
    private boolean hasDiscount;

public void add(ItemOrder itemOrder){
        if(cart.contains(itemOrder.getItem())){
            int index = cart.indexOf(itemOrder.getItem());
            cart.set(index, itemOrder);
        } else cart.add(itemOrder);
 }

My code just adds the price of the old order with the new one, for example changing the ItemOrder from 3 to 5 (each item for $1) gives the price of 8 items ($8) when it is supposed to change to price from the price of 3 ($3) to ($5)
so what is wrong with it ?

this is supposed to add an order of the item, if the item is previously ordered then it must substitute it with the new order, thus the total price showing the price of the new order and not adding the previous and new one.

        private Item orderItem;

        public double getTotal(){
        double totalPrice = 0;
        for(int i = 0; i < cart.size(); i++){
            totalPrice += cart.get(i).getPrice();
        }
        if(hasDiscount)
            totalPrice *= 0.9;
        return totalPrice;
    }

    public Item getItem(){
       return  this.orderItem;
    }

this is the method that calculates the total price of the order.

You may want to look at these classes as well :

public double getPrice(){
        return orderItem.priceFor(orderQuantity);
    }

    private String itemName;
    private double itemPrice;
    private double itemBulkPrice;
    private int itemBulkQuantity;

/**
     * This method return the price of the item
     * with considering the bulk price if it exists
     * @param int quantity to calculate its price
     */
    public double priceFor (int quantity)
    {
        if(quantity < 0)
            throw new IllegalArgumentException("Price you entered is negative");
        double totalPrice = 0;
        if (itemBulkQuantity > 0 ){
            int numOfBulk = quantity / itemBulkQuantity;
            int remainder = quantity % itemBulkQuantity;
            totalPrice = (numOfBulk * itemBulkPrice) + (remainder * itemPrice);
        } else 
            totalPrice = quantity * itemPrice;
        return totalPrice; 
    }

[baf06]

It seems that i didn't explain my problem clearly, so this is a sample picture of what is going wrong



the second picture is when you clear the order of the 3 silly putty with 5, so the total price was calculates to be the price of the 8 and not the five as it is supposed to give (like in the 3rd picture, the right side)

hope i cleared my problem, i am trying many ways to fix this but i am not finding a solution; but i know that the problem is in the add(ItemOrder) method in the ShoppingCart class, so hopefully i can get some help or hints to fix this out.
Thanks in advance,

[santeron]

Your code seems fine for the last 5mins I look at it... Maybe you could post the whole class so we can figure out what all the methods are doing.

public double getTotal(){
        double totalPrice = 0;
        for(int i = 0; i < cart.size(); i++){
            totalPrice += cart.get(i).getPrice();
        }

If you want the total price shouldn't you write something like: totalPrice += car.get(i).getQuantity() * cart.get(i).getPrice() or the getPrice() has calculated the quantity too?

[baf06]

yes the getPrice() does this

public double getPrice(){
        return orderItem.priceFor(orderQuantity);
    }

and the priceFore(int) calculate the price depending on the qauntity

[baf06]

These are my classes:

/**
 * This class stores the information
 * about an individual item
 */

import java.text.*;
public class Item
{
    private String itemName;
    private double itemPrice;
    private double itemBulkPrice;
    private int itemBulkQuantity;
    
    /**
     * This method creates an item throwing an exception if price is negative
     * @param String name of item 
     * @param double price of the item
     */
    public Item (String name, double price)
    {        
        if(price < 0)
            throw new IllegalArgumentException("Price you entered is negative");
        itemName = name;
        itemPrice = price;
    }
    
    /**
     * This method creates an item throwing an exception if price is negative
     * @param String name of item 
     *      double price of the item
     *      int Bulk Quantity
     *      double bulk price
     */
    public Item (String name, double price, int bulkQuantity, double bulkPrice)
    {
        if(price < 0 || bulkQuantity < 0 || bulkPrice < 0)
            throw new IllegalArgumentException("Price you entered is negative");
        itemName = name;
        itemPrice = price;
        itemBulkQuantity = bulkQuantity;
        itemBulkPrice = bulkPrice;
    }
    
    /**
     * This method return the price of the item
     * with considering the bulk price if it exists
     * @param int quantity to calculate its price
     */
    public double priceFor (int quantity)
    {
        if(quantity < 0)
            throw new IllegalArgumentException("Price you entered is negative");
        double totalPrice = 0;
        if (itemBulkQuantity > 0 ){
            int numOfBulk = quantity / itemBulkQuantity;
            int remainder = quantity % itemBulkQuantity;
            totalPrice = (numOfBulk * itemBulkPrice) + (remainder * itemPrice);
        } else 
            totalPrice = quantity * itemPrice;
        return totalPrice; 
    }
    
    /**
     * This method writes the item as a string of a certain form
     */
    public String toString(){
        NumberFormat prices = NumberFormat.getCurrencyInstance();
        String str = itemName + ", " + prices.format(itemPrice);
        if (itemBulkPrice > 0){
            str += " (" + itemBulkQuantity + " for " + prices.format(itemBulkPrice) + ")";
        }
        return str;
    }
}

/**
 * This class stores information about a collection of items
 */

import java.util.*; 
public class Catalog
{
    private ArrayList<Item> catalogItems;
    private String catalogName;

    /**
     * Constructor that creates a list of Item objects
     * @param String the name of the catalog to be formed
     */
    public Catalog(String name){
        catalogName = name;
        catalogItems = new ArrayList<Item>();
    }

    /**
     * Method that adds an Item to the end of the list in the catalog
     * @param name of the Item to be added
     */
    public void add(Item item){
        catalogItems.add(item);
    }

    /**
     * @return the size of the catalog
     */
    public int size(){
        return catalogItems.size();
    }

    /**
     * Method that return the wanted Item object
     * @param int index of the Item from the catalog
     */
    public Item get(int index){
        return catalogItems.get(index);
    }

    /**
     * @return the name of this catalog
     */
    public String getName(){
        return catalogName;
    }
}

/**
 * This class stores the information about a particular item and the
 * quantity ordered for that item
 */

import java.util.*;
public class ItemOrder
{
    private int orderQuantity;
    private Item orderItem;
    
    /**
     * Constructor that creates an item order for the given item
     * @param Item the ordered item
     * @param int the quantity order from the item
     */
    public ItemOrder(Item item, int quantity){
        orderItem = item;
        orderQuantity = quantity;
    }

    /**
     * @return the total cost of the item order
     */
    public double getPrice(){
        return orderItem.priceFor(orderQuantity);
    }

    /**
     * @return a reference to the Item object of the item order
     */
    public Item getItem(){
       return  this.orderItem;
    }
}

/**
 * This class stores information about the overall order
 * and whether the order gets a discount or not
 */

import java.util.*; 
public class ShoppingCart
{
    private ArrayList<ItemOrder> cart;
    private boolean hasDiscount;

    /**
     * Constructor that creates an empty ArrayList of ItemOrder
     */
    public ShoppingCart(){
        cart = new ArrayList<ItemOrder>();
    }

    /**
     * Method that adds an ItemOrder to the list
     * if the list contains any previous order of this item
     * it replaces it with the new order
     * @param ItemOrder the order to be added
     */
    public void add(ItemOrder itemOrder){
        if(cart.contains(itemOrder.getItem())){
            int index = cart.indexOf(itemOrder.getItem());
            cart.set(index, itemOrder);
        } else cart.add(itemOrder);
    }

    /**
     * Method that sets whether or not this order gets a discount
     * @param boolean that determines if it gets a discount
     */
    public boolean setDiscount(boolean value){
        return hasDiscount = value;
    }

    /**
     * @return the total cost of the shopping cart
     * taking into consideration if the order gets a discount
     */
    public double getTotal(){
        double totalPrice = 0;
        for(int i = 0; i < cart.size(); i++){
            totalPrice += cart.get(i).getPrice();
        }
        if(hasDiscount)
            totalPrice *= 0.9;
        return totalPrice;
    }
}

[baf06]

I still have the main class and another one but they are correct since my instructor gave them to me, so these are the one that concerns the problem i am facing,
thanks alot

[dlorde]

I don't understand your ShoppingCart.add(..) method. The 'cart' is an ArrayList of ItemOrder elements and the add method takes an ItemOrder argument. But instead of looking for the argument ItemOrder in the cart list of ItemOrders, it looks for argument ItemOrder's Item. This won't be found, because the list contains ItemOrders not Items. It seems to me that the add method will always add the ItemOrder to the list, because of this.

[baf06]

yup you are right, and i fixed it to search for the itemOrder and not the item, but still the problem occurs,
this method is supposed to replace an existing itemOrder of the same order with the new order, thus changing the totalPrice, i tried several ways but it didn't work out

public void add(ItemOrder itemOrder){
        if(cart.contains(itemOrder)){
            int index = cart.indexOf(itemOrder);
            cart.set(index, itemOrder);
        } else cart.add(itemOrder);
    }

[dlorde]

Are you sure that the ItemOrder object you are searching for is the exact same object that was originally stored in the cart (it seems unlikely if the quantity is different)?

If you want to find an object in a collection by some internal value it has, you must override the equals(..) method (and, ideally, the hashcode() method).

If you want to find ItemOrder objects that have the same Item in them, you'll need to compare the Items in the ItemOrder.equals method - presumably by Item name, or you could give Item itself an equals method.

However, I would think carefully before giving ItemOrder an equals method that is true if the Item is the same. This would mean that ItemOrders with different quantities for the same Item would compare the same - fine for looking up in the cart, but might be problematic elsewhere.

You may have found a design problem - it often happens at this stage. An alternative would be to use a map to store ItemOrder in the cart, with the Item as the key. This way, you can search the map for the Item and if found, replace its mapped ItemOrder.

[santeron]

Just by reading the code I can't get to a conclusion tbh. Seems pretty neat. Maybe you should try throwing some System.out.prints in there and follow the values in different classes and see how it's changed. One little tip, in your constructors add an equal to every condition you check because you don't want zero values on prices and quantities too ;)

Maybe I could help if you gave the other two classes so I could compile and see what happens...

PS. dlorde may be right on this... You probably never get equal objects and you just add them ;) http://bytes.com/topic/java/insights...shcode-methods

[baf06]

these tips are really good and helpful really, but the instructor said that is less likely to need to form any extra method or class, but I will try fixing it by adding a method Item.equals
and if i faced any problems i will ask, thanks very much for the help

[baf06]

santeron these are the two codes :

/**
 * Class ShoppingFrame provides the user interface for a simple shopping
 * program.
 */
import javax.swing.*;
import java.awt.*;
import java.awt.event.*;
import java.text.*;

public class ShoppingFrame extends JFrame {
    private ShoppingCart items;
    private JTextField total;

    public ShoppingFrame(Catalog products)      {
        // create frame and order list
        setTitle(products.getName());
        setDefaultCloseOperation(EXIT_ON_CLOSE);
        items = new ShoppingCart();

        // set up text field with order total
        total = new JTextField("$0.00", 12);
        total.setEditable(false);
        total.setEnabled(false);
        total.setDisabledTextColor(Color.BLACK);
        JPanel p = new JPanel();
        p.setBackground(Color.blue);
        JLabel l = new JLabel("order total");
        l.setForeground(Color.YELLOW);
        p.add(l);
        p.add(total);
        add(p, BorderLayout.NORTH);

        p = new JPanel(new GridLayout(products.size(), 1));
        for (int i = 0; i < products.size(); i++)
            addItem(products.get(i), p);
        add(p, BorderLayout.CENTER);

        p = new JPanel();
       add(makeCheckBoxPanel(), BorderLayout.SOUTH);

        // adjust size to just fit
        pack();
    }

    // Sets up the "discount" checkbox for the frame
    private JPanel makeCheckBoxPanel() {
        JPanel p = new JPanel();
        p.setBackground(Color.blue);
        final JCheckBox cb = new JCheckBox("qualify for discount");
        p.add(cb);
        cb.addActionListener(new ActionListener() {
            public void actionPerformed(ActionEvent e) {
                items.setDiscount(cb.isSelected());
                updateTotal();
            }
        });
        return p;
    }

    // adds a product to the panel, including a textfield for user input of
    // the quantity
    private void addItem(final Item product, JPanel p) {
        JPanel sub = new JPanel(new FlowLayout(FlowLayout.LEFT));
        sub.setBackground(new Color(0, 180, 0));
        final JTextField quantity = new JTextField(3);
        quantity.setHorizontalAlignment(SwingConstants.CENTER);
        quantity.addActionListener(new ActionListener() {
            public void actionPerformed(ActionEvent e) {
                updateItem(product, quantity);
                quantity.transferFocus();
            }
        });
        quantity.addFocusListener(new FocusAdapter() {
            public void focusLost(FocusEvent e) {
                updateItem(product, quantity);
            }
        });
        sub.add(quantity);
        JLabel l = new JLabel("" + product);
        l.setForeground(Color.white);
        sub.add(l);
        p.add(sub);
    }

    // When the user types a new value into one of the quantity fields,
    // parse the input and update the ShoppingCart.  Display an error
    // message if text is not a number or is negative.
    private void updateItem(Item product, JTextField quantity) {
        int number;
        String text = quantity.getText().trim();
        try {
            number = Integer.parseInt(text);
        } catch (NumberFormatException error) {
            number = 0;
        }
        if (number <= 0 && text.length() > 0) {
            Toolkit.getDefaultToolkit().beep();
            quantity.setText("");
            number = 0;
        }
        items.add(new ItemOrder(product, number));
        updateTotal();
    }

    // reset the text field for order total
    private void updateTotal() {
        double amount = items.getTotal();
        total.setText(NumberFormat.getCurrencyInstance().format(amount));
    }
}

/**
 * ShoppingMain provides method main for a simple shopping program.
 */

public class ShoppingMain {
    public static void main(String[] args) {
        Catalog list = new Catalog("CS Gift Catalog");
        list.add(new Item("silly putty", 3.95, 10, 19.99));
        list.add(new Item("silly string", 3.50, 10, 14.95));
        list.add(new Item("bottle o bubbles", 0.99));
        list.add(new Item("Nintendo Wii system", 389.99));
        list.add(new Item("Mario Computer Science Party 2 (Wii)", 49.99));
        list.add(new Item("Don Knuth Code Jam Challenge (Wii)", 49.99));
        list.add(new Item("Computer Science pen", 3.40));
        list.add(new Item("Rubik's cube", 9.10));
        list.add(new Item("Computer Science Barbie", 19.99));
        list.add(new Item("'Java Rules!' button", 0.99, 10, 5.0));
        list.add(new Item("'Java Rules!' bumper sticker", 0.99, 20, 8.95));

        ShoppingFrame f = new ShoppingFrame(list);
        f.setVisible(true);
    }
}

[baf06]

I previously used to override the equal method but i didn't think of this solution on this problem, but thanks for the tip :)

[santeron]

Override equals and hashcode. This should be your problem as dlorde stated. If the problem insists I'll be happy to help you again :)

[baf06]

yes it worked the way you told me, thanks very much :) this is what i changed

in the ShoppingCart class

public void add(ItemOrder the_order)
    {    
        for(int i = 0; i < cart.size(); i++)
        {
            if(the_order.getItem().equals(cart.get(i).getItem()))
            {
                cart.set(i, the_order);
                return;
            }
        }
        cart.add(the_order);
    }

and in the Item class

public boolean equals(final Object some_other)
    {
        boolean result = false;
        if (this == some_other)
        {
            result = true;
        }
        else if ((some_other != null) && (some_other.getClass() == getClass()))
        {
            final Item some_item = (Item) some_other;

            //compare all fields in item when comparing two objects. this will be true
            //only when all of the fields are the same.
            result = 
            ((some_item.itemBulkPrice == itemBulkPrice) && 
                (some_item.itemBulkQuantity == itemBulkQuantity) && 
                (some_item.itemPrice == itemPrice) && 
                (some_item.itemName.equals(itemName)));
        }
        return result;
    }

    public int hashCode()
    {
        return toString().hashCode();
    }

this change in hashCode is correct write? because i never used it before

[dlorde]

Iterating through the shopping cart will work, but if you use a map instead of a list, you won't need to iterate at all:

private Map<Item, ItemOrder> cart = new HashMap<Item, ItemOrder>();
...
public void add(ItemOrder itemOrder){
    cart.put(itemOrder.getItem(), itemOrder);  // replaces existing Item mapping or adds a new one
}

.
Hashcode should give you a unique identifier based on the same criteria used in the equals(..) method. Using toString() is OK unless you override the toString() method and don't use the same items as the equals(..) method. Not very reliable - its equivalent to comparing the toString() result for your equals(..) method...

A popular Java IDE (IntelliJ IDEA) generates the following hashCode method for your Item class:

public int hashCode() {
    int result;
    long temp;
    result = itemName != null ? itemName.hashCode() : 0;
    temp = itemPrice != +0.0d ? Double.doubleToLongBits(itemPrice) : 0L;
    result = 31 * result + (int) (temp ^ (temp >>> 32));
    temp = itemBulkPrice != +0.0d ? Double.doubleToLongBits(itemBulkPrice) : 0L;
    result = 31 * result + (int) (temp ^ (temp >>> 32));
    result = 31 * result + itemBulkQuantity;
    return result;
}

[baf06]

Thanks for this detailed code but this work is to practice ArrayList so I am not supposed to work on Map, and another point it that there are some codes in the hashCode you gave that we didn't take yet at university so i prefer not to use all this amount if upgraded codes, but tried to understand every thing in it, so thanks for the extra information :)

[dlorde]

No problem, you're welcome.

It was just an example - you don't actually need a fancy hashCode method in Item if you're not going to store the Item objects themselves in a collection.