Duplicate Value insertion in Hashmap

Question

There is a Hashmap (o) which takes string as a key and Order Object as value. Order has an Arraylist of OrderLines. Here I have to add multiple orders to the map. The problem is my hashmap prints out unique first and second keys (order 1 and Order 2) but the last inserted value as value for both keys (duplicate order in all entries). Can you please help me debug the problem ?

Main Class :

    Map<String, Order> o = new HashMap<String, Order>();

    Order c = new Order();

    c.add(new OrderLine(new Item("book", (float) 12.49), 1));
    c.add(new OrderLine(new Item("music CD", (float) 14.99), 1));

    o.put("Order 1", c);

    // Reuse cart for an other order
    c.clearCart(); // this.orderLines.clear() in the order class 

    c.add(new OrderLine(new Item("imported box of chocolate", 10), 1));
    c.add(new OrderLine(new Item("imported bottle of perfume", (float)     47.50), 1));

    o.put("Order 2", c);

    for (Map.Entry<String, Order> entry : o.entrySet()) {
        System.out.println("*******" + entry.getKey() + entry.getValue().get(0).getItem().getDescription() + "*******");
    }

Order class :

class Order {

private List<OrderLine> orderLines = new ArrayList<>();

public void add(OrderLine o) throws Exception {
    orderLines.add(o);
}

public OrderLine get(int i) {
    return orderLines.get(i);
}

public void clearCart() {
    this.orderLines.clear();
}
}

OrderLine class:

private int quantity;
private Item item;

public OrderLine(Item item, int quantity) throws Exception {
    if (item == null) {
        System.err.println("ERROR - Item is NULL");
        throw new Exception("Item is NULL");
    }
    assert quantity > 0;
    this.item = item;
    this.quantity = quantity;
}

public Item getItem() {
    return item;
}

public int getQuantity() {
    return quantity;
}
}

Item class:

class Item {

    private String description;
    private float price;

    public Item(String description, float price) {
        super();
        this.description = description;
        this.price = price;
    }

    public String getDescription() {
        return description;
    }

    public float getPrice() {
        return price;
    }
    }

where is OrderLine class. That's how your code does. Post entire code, I'll suggest fix. Looks like c.clearCart() is not able to clear cart — JavaHopper, Aug 05 '16 at 03:53
How do you get `entry.getValue().get(0)` at the SOP in the for loop in the main class ? — Surajit Biswas, Aug 05 '16 at 04:05
See my answer. This question should not be downvoted like it is. There is more than enough code to understand the issue and the answer actually lies within Java's behind-the-scene workings. It is perfectly understandable that the OP could not find the answer as he did not know the question. — Alexander Guyer, Aug 05 '16 at 04:09
The order which is used as Value object in map has a get(i) method which returns the specific orderline @SurajitBiswas — Rk R Bairi, Aug 05 '16 at 04:21

score 2 · Accepted Answer · edited May 23 '17 at 10:28

While Java is pass-by-value, it is perfectly valid to alter what's at the end of a reference. This is what you are doing, albeit inadvertently.

Think about what you're doing: you're adding c to the list, then clearing c, re-initializing it, and adding it again.

As you never use the new keyword, you're never actually allocating a new portion of memory for c. It is still pointing to the same Order. Simultaneously, you did not add a clone of c to the list. You added c.

In other words, when you call c.clearCart(), you're also clearing the first Order in list o as said Order is c.

You could either use the new keyword by replacing:

c.clearCart();

with

c = new Order();

Or, you could add a clone of c to list o instead of c itself, so that when you call c.clearCart(), you are not clearing the first element in list o. In other words, replace:

o.put("Order 1", c);

with

o.put("Order 1", c.clone());

See this question for more information.

EDIT:

I forgot one part, though it may be obvious. I stated that clearing c will also clear the first element in list o, because that element is c. However, I forgot to mention that, by transition, re-initializing c also means re-initializing this element simultaneously. Consequently, when you add c again, you have two elements initialized with the same fields.

Duplicate Value insertion in Hashmap

1 Answers1