Search This Blog

Saturday, 7 November 2015

Be careful with Sets and equals behavior

I was recently assigned to fix a bug in the code. For the scope of this post I re-framed the problem as below:
Context: There is a set of objects of each type (lets say fruits). Over a series of operations different type of fruits are added to the set. In certain stages certain fruits may be replaced by others in the family. For example during stage 4, we may need to replace a green apple by a red apple. On the final page we display all the fruits selected.
Problem: While the various fruits show up, the changes are not seen. So if we had updated the apple to be a red one, the final result still displays a green apple.
If the code for the same were to be written out I would have a fruit class:
public class Fruit {

  private String name;
  private String color;
  private int cost;

  @Override
  public int hashCode() {
    final int prime = 31;
    int result = 1;
    result = prime * result + ((name == null) ? 0 : name.hashCode());
    return result;
  }

  @Override
  public boolean equals(Object obj) {
    if (this == obj)
      return true;
    if (obj == null)
      return false;
    if (getClass() != obj.getClass())
      return false;
    Fruit other = (Fruit) obj;
    if (name == null) {
      if (other.name != null)
        return false;
    } else if (!name.equals(other.name))
      return false;
    return true;
  }

  // setter getters here 
}
As seen here fruits are unique by their name. Thus the set can only hold one fruit of each type.
Now the main code, where we modify the set in stages
public static void main(String[] args) {
    Set<Fruit> fruitBasket = new HashSet<>();
    //Stage 1----------
    fruitBasket.add(new Fruit("Apple", "green", 3));
    fruitBasket.add(new Fruit("Watermelon", "green", 11));
    
    //Stage 2-------------------
    //replace a green apple by a red one
    fruitBasket.add(new Fruit("Apple", "red", 3));
    fruitBasket.add(new Fruit("Mango", "yellow", 5));
    
    //Final Display
    System.out.println("The fruits in the basket are");
    for (Fruit fruit : fruitBasket) {
      System.out.println(fruit.getColor() + " colored " +  fruit.getName() + " costs " + fruit.getCost());
    }
    
  }
If I were to run this code I would expect to see red Apple, yellow Mango and a red watermelon.
The fruits in the basket are
green colored Apple costs 3
green colored Watermelon costs 11
yellow colored Mango costs 5
There's been a mistake ! I expected a red Apple not a green one. What was the bug in the above code ??

If we look at the behavior of the Set's add method then:
public boolean add(E e) {
        return map.put(e, PRESENT)==null;
    }
This seems fine. We know that the HashSet internally uses a Map. The add method tries to add the passed Fruit as a key entry in the underlying map. But if we look at the documentation
Adds the specified element to this set if it is not already present (optional operation).
More formally, adds the specified element e to this set if the set contains no element 
e2 such that (e==null ? e2==null : e.equals(e2)). If this set already contains the 
element, the call leaves the set unchanged and returns false. 
This is where things get interesting. Two Fruits are same if they have the same name. Thus for the set, the Green Apple and the Red Apple are identical. The Set will ignore the add request for the Red Apple and hence we see the Green Apple in the result. To fix the same, I changed the add code as below :
public static void main(String[] args) {
    Set<Fruit> fruitBasket = new HashSet<>();
    //Stage 1----------
    
    
    //fruitBasket.add(new Fruit("Apple", "green", 3));
    add(fruitBasket, new Fruit("Apple", "green", 3));
    //fruitBasket.add(new Fruit("Watermelon", "green", 11));
    add(fruitBasket, new Fruit("Watermelon", "green", 11));
    //Stage 2-------------------
    //replace a green apple by a red one
   // fruitBasket.add(new Fruit("Apple", "red", 3));
    add(fruitBasket, new Fruit("Apple", "red", 3));
//    fruitBasket.add(new Fruit("Mango", "yellow", 5));
    add(fruitBasket, new Fruit("Mango", "yellow", 5));
    //Final Display
    System.out.println("The fruits in the basket are");
    for (Fruit fruit : fruitBasket) {
      System.out.println(fruit.getColor() + " colored " +  fruit.getName() + " costs " + fruit.getCost());
    }    
  }
  
  public static void add(Set<Fruit> fruitBasket, Fruit crtFruit) {
    if(fruitBasket.contains(crtFruit)) {
      fruitBasket.remove(crtFruit);
    }
    
    fruitBasket.add(crtFruit);
  }
This code will now work fine. We will first remove the Apple if it exists before adding the new/modified apple. The output on running is
The fruits in the basket are
red colored Apple costs 3
green colored Watermelon costs 11
yellow colored Mango costs 5
This is interesting when you compare with Maps. In the map if a key exists, then its value field is updated with the parameters in the current request. But not for sets. You need to explicitly remove and add again.

No comments:

Post a Comment