Properly overriding equals and hashCode

[Andrew Mcmurray]

Hi all ,

I have been seeing some stuff on how important it is to properly override the equals and hashCode methods and how most of the time people do it wrong. I want to make sure I am doing it right, right meaning conforming to the general contract of the equals and hashCode methods.

I have two classes Item and ShoppingCart. Item is considered equal if the description and cost are the same and Shopping Cart is considered equal both carts contain the exact same items. Here are my overriden equals and hashCode methods. Let me know if they look ok or that they need some fixing?

Thanks,

AMD

//Item equals and hashCode

 public boolean equals(Object obj)
    {
      boolean retVal = false;
     
      if(this == obj)
      {
    retVal =  true;
      }
      //Prevents a subclass from violating the symmetry requirement 
      if((obj == null) || (obj.getClass() != this.getClass()))
      {
    retVal = false;
      }
      else
      {  
    // object must be Item at this point
    Item item = (Item)obj;
    if( cost == item.cost && (description == item.description || (description != null && description.equals(item.description))))
    {
      retVal = true;
    }
    else
    {
      retVal = false;
    }
      }
         return retVal;
    }
    
    
    public int hashCode()
    {
      int hash = 7;
      hash = 31 * hash + (int)cost;
      hash = 31 * hash + (null == description ? 0 : description.hashCode());
      
      return hash;
    }

//ShoppingCart methods almost the same as above

public boolean equals(Object obj)
    {
      boolean retVal = false;
     
      if(this == obj)
      {
    retVal =  true;
      }
      //Prevents a subclass from violating the symmetry requirement 
      if((obj == null) || (obj.getClass() != this.getClass()))
      {
    retVal = false;
      }
      else
      {  
    // object must be Item at this point
    ShoppingCart cart = (ShoppingCart)obj;
    
    //If both Carts are empty
    if((cart.items.isEmpty() && this.items.isEmpty()))
    {
     retVal = true;
    }
    
    if(cart.items.size() == this.items.size())
    {
     for(int i=0; i<this.items.size(); i++)
     {
       if(cart.items.get(i).equals(this.getItems().get(i)))
       {
    retVal = true;   
       }
       else
         {
          retVal = false;
          break;
         }  
     }
    }
    else
    {
     retVal = false;
    }
  
      }
         return retVal;
    }
    
    
    public int hashCode()
    {
      int hash = 7;
      hash = 31 * hash + (null == items ? 0 : items.hashCode());
      return hash;
    }
 

[Jeff Albertson]

If items is a List, you can use equals. See the definition of List's equals method:

retValue = Util.equals(items, cart.items);
 
public class Util{
    ...
 
    public static boolean equals(Object a, Object b) {
        return a==null ? b==null: a.equals(b);
    }
}

What is cost's type? It looks like it may be float or double. Realize that storing currency in a floating point type is always asking for trouble because of rounding. Consider storing it in an integer type (cents or tenths of cents, etc...) or using BigDecimal or your own custom class. Back to float/double: do you know they have one last trick up there sleeve? floating point numbers can take on the value NaN (not a number) which doesn't obey the usual rules of the relational operators, for example, Double.Nan == Double.NaN is false! Of course, your cost probably never takes on that value, but the paranoid code for that in equals/hasCode.
[ February 03, 2006: Message edited by: Jeff Albertson ]

[Andrew Mcmurray]

Thanks Jeff,

Other then that does it look ok?

Thanks,

AMD

[Paul Clapham]

I think your ShoppingCart's hashCode() method returns different values for two objects whose items are the same overall but in different orders. But then its equals() method returns false for two such lists, so technically that is okay.