java HashMap not updating integer value for a specific key

Question

package cdlKata;

import java.util.HashMap;
import java.util.Map;

class Item2 {
  private String s;
  public Item2(String s) { this.s = s; }
  public String getS() { return s; }
}

class Basket2 {
  private Map<Item2, Integer> items;

  public Basket2() { items = new HashMap<>(); }

  public Map<Item2, Integer> getItems() { return items; }

  public void addItemToBasket(Item2 item) {
    items.put(item, items.getOrDefault(item,0) + 1);
  }

  public void printBasket() {
    items.entrySet().forEach(e->{ System.out.println(e.getKey().getS() + " " + e.getValue());});
  }
}

public class Main2 {

  public static void main(String[] args) {
    Basket2 basket;
    basket = new Basket2();
    basket.addItemToBasket(new Item2("A"));
    basket.addItemToBasket(new Item2("A"));
    basket.printBasket();
  }
}

Result is :

A 1
A 1

with basket size = 2. What I want is :

A 2

with basket size 1.

if I turn Item2 into a String I got no issue. Don't understans why it is not working.

oh gosh is it because of this? Item2 item1 = new Item2("A"); Item2 item2 = new Item2("A"); System.out.println("??" + item1.equals(item2)); -> FALSE — Francesco Dassisis, May 16 '20 at 06:10
Implementing hashCode/equals might not be the best solution, as you must not include the number of items in the comparison. This might get you into trouble latter on. Thus I’d follow the answer by @BhushanWawre — dpr, May 16 '20 at 06:47
@dpr Could you please enlighten me? Why it is not best practice store objects in map? What trouble will come later? — Gibbs, May 18 '20 at 07:51
@Gibbs obviously to use the object as key in the map it must not include the quantity in the equals/hashCode implementation. But in other parts of the program it is very likely that two items with the same name but different quantity should not be considered equal. As there can only be one equals method I'd recommend not to use business objects as map keys... — dpr, May 18 '20 at 07:57

Gibbs · Answer 1 · 2020-05-16T06:17:47.133

1

You created two different instances as below

basket.addItemToBasket(new Item2("A"));
basket.addItemToBasket(new Item2("A"));

You are expecting it to be equal. It will not be the case.

You have to override hashcode and equals

edited May 16 '20 at 06:17

answered May 16 '20 at 06:08

Gibbs

21,904
13
74
138

that's correct.. I wrote it in my first comment... realised it just after I posted this.. thanks though – Francesco Dassisis May 16 '20 at 06:47

score 1 · Answer 2 · answered May 16 '20 at 06:13

1

According to the logic of work of HashMap, you must override your keys' hashcode() and equals() methods. P.S. Also I would suggest you to use in this case merge method of map instead of put.

map.merge(key, 1, Integer::sum)

answered May 16 '20 at 06:13

Andrew Kolesnyk

161
6

score 1 · Answer 3 · answered May 16 '20 at 06:27

Since you are using Map of object,Integer the output :

A 1

is correct, but you are expecting output: A 2

for which you need to create Map of String,Integer ie. map of item2.getS() and Integer.

The reason it is not working is because you Mapped object and Integer, and you are adding item to basket by creating new objects each time.

So, the solution will be like this.

    package cdlKata;

    import java.util.HashMap;
    import java.util.Map;

    class Item2 {
        private String s;
        public Item2(String s) { this.s = s; }
        public String getS() { return s; }
    }

    class Basket2 {
        private Map<String, Integer> items;

        public Basket2() { items = new HashMap<>(); }

        public Map<String, Integer> getItems() { return items; }

        public void addItemToBasket(Item2 item) {
            items.put(item.getS(), items.getOrDefault(item.getS(),0) + 1);
        }

        public void printBasket() {
            items.entrySet().forEach(e->{ System.out.println(e.getKey() + " " + e.getValue());});
        }
    }

    public class Stack {

        public static void main(String[] args) {
            Basket2 basket;
            basket = new Basket2();
            basket.addItemToBasket(new Item2("A"));
            basket.addItemToBasket(new Item2("A"));
            basket.printBasket();
        }
    }

score 0 · Answer 4 · answered May 16 '20 at 06:11

Well... You are missing equals/hashCode in your class.

See this answer for more info: Why do I need to override the equals and hashCode methods in Java?

For exactly your class just define these methods:

class Item2 {
    private String s;
    public Item2(String s) { this.s = s; }
    public String getS() { return s; }


    @Override public boolean equals(Object o)
    {
        if (this == o)
            return true;
        if (o == null || getClass() != o.getClass())
            return false;

        Item2 item2 = (Item2) o;

        return s != null ? s.equals(item2.s) : item2.s == null;
    }


    @Override public int hashCode()
    {
        return s != null ? s.hashCode() : 0;
    }
}

score 0 · Answer 5 · answered May 16 '20 at 06:11

Is is a common mistake of using hash. You should implement hashCode and equals for Item class.

class Item2 {
  private String s;
  public Item2(String s) { this.s = s; }
  public String getS() { return s; }

  @Override
  public boolean equals(Object o) {
    if (this == o) return true;
    if (!(o instanceof Item2)) return false;
    Item2 item2 = (Item2) o;
    return Objects.equals(getS(), item2.getS());
  }

  @Override
  public int hashCode() {
    return Objects.hash(getS());
  }
}

Without that, HashMap will consider two objects (even the same value) are different, Therefore, it will put as a new Object.

java HashMap not updating integer value for a specific key

5 Answers5