Override equals and hashCode

[Tom Henricksen]

I have a custom class I created and needed to override equals and hashCode.

    
public boolean equals(Object obj) {
if (obj != null && obj instanceof Machine) {
Machine machine = (Machine) obj;
if (machine.getComplexId().equals(this.getComplexId())) {
if (machine.getComplexId().equals(this.getComplexId())) {
return true;
}
}
}
return false;
}
 
public int hashCode() {
        int hash = 1;
hash = hash * 31 + this.complexId.hashCode();
hash = hash * 31 + (this.machineTypeId == null ? 0 :   this.machineTypeId.hashCode());
return hash;
}

Does this look okay?
Thanks,
Tom
[ December 21, 2006: Message edited by: Tom Henricksen ]

[Ernest Friedman-Hill]

1) Why are there two nested sets of "if (machine.getComplexId().equals(this.getComplexId()))"?

2)Most equals() implementations start with "if (this == obj) return true;" as an optimization for a common case.

3) This is actually not OK, at least in theory. The hashCode method uses two different members to compute the hashCode(), but equals() uses only one; that means that it's theoretically possible for two Machines to be equal, but have different hashCode()s. Perhaps this is related to #1 -- maybe one of those nested conditions is supposed to refer to the other member.

[Edwin Dalorzo]

My comments on equals:

The same attributes you take into account in equals() you must take into account in hashCode(). However in your hashCode() you use machineTypeId, but you did not in you equals() method.

There is not need to check for the object being null. Instanceof operator will return false if the object is null.

Test first for object equality (same instance) and avoid further complications in the algorithm.

public boolean equals(Object obj) {
 
   if(obj==this)
     return true;
   
  if (obj instanceof Machine) {
     Machine machine = (Machine) obj;
     boolean result = false;
     
     result = complexId = machine.complexId || (complexId != null && complexId.equals(machine.complexId);
     
     result &=  machineTypeId = machine.machineTypeId || (machineTypeId != null && machineTypeId.equals(machine.machineTypeId);
 
     return result;
  } 
  return false;
}

My comments on hashCode:

You can use the members hashCode, but always test for nullability first.

public int hashCode() {        
   int hash = 1;
   hash = hash * 31 + (complexId == null ? 0 : complexId.hashCode();
   hash = hash * 31 + (machineTypeId == null ? 0 :   this.machineTypeId.hashCode());
  
   return hash;
}

I hope that helps!
[ December 21, 2006: Message edited by: Edwin Dalorzo ]

[Tom Henricksen]

Thank you very much for this advice. I appreciate it very much.

Happy Holidays!

Thanks,
Tom