Francesco Dassisis
Francesco Dassisis

Reputation: 142

java HashMap not updating integer value for a specific key

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.


Upvotes: 0

Views: 542

Answers (5)

Bhushan Wawre
Bhushan Wawre

Reputation: 21

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

A 1

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();
        }
    }

Upvotes: 1

Gibbs
Gibbs

Reputation: 22974

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

Upvotes: 1

Andrew Kolesnyk
Andrew Kolesnyk

Reputation: 211

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)

Upvotes: 1

Peter Wang
Peter Wang

Reputation: 116

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.

Upvotes: 0

Sergey Ushakov
Sergey Ushakov

Reputation: 186

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;
    }
}

Upvotes: 0

Related Questions