Override equals method

Question

newbie question here:

So in my university homework I have to override the object class equals method for a new class created by me.

The new class is "Product", each product has an "id" attribute which is unique. So this is how I Overrided it:

@Override
public boolean equals(Object obj) {
       final Product other = (Product) obj;
       if (id != other.id)
           return false;
       return true;
   }

The thing is that doing this is 1,5 points out of 10 and it made me suspicius to be that easy. So i started searching and I found things like:

@Override
public boolean equals(Object obj) {
       if (this == obj)
           return true;
       if (obj == null)
           return false;
       if (getClass() != obj.getClass())
           return false;
       final Product other = (Product) obj;
       if (id != other.id)
           return false;
       return true;
   }

Which don't make sense for me at all, because I think that the last if check all the other ifs restrictions. What do you guys think?Which is the better way to Override this method?

Thanks!

Solution 1

The second piece of code is better:

• It optimizes for x.equals(x), which isn't necessary for correctness, but is a helpful optimization
• It copes with x.equals(null) instead of throwing NullPointerException
• It handles objects of a completely different class without throwing a ClassCastException which yours would (e.g. x.equals("foo"))
• It requires the exact same type to provide a symmetric relationship; otherwise obj.equals(x) could invoke a different method, giving a different result.

Solution 2

The second version is a safe one, I would say a pedantic one. Your version, instead, could launch a ClassCastException because you are assuming that the runtime type of the variable obj is of type product. Which is not true, that's why you should use this.getClass() != obj.getClass() (you could solve this problem also with instanceof operator).

If I do

Product p = new Product();
p.equals("abc");

I get an exception while I should get false.

In addition it manages the product.equals(null) problem, which should return false as stated in equals contract method in documentation. If you don't care about this and you do, inside you equals:

...
Product p = (Product)obj; // obj is null
obj.id // this throws a NullPointerException

Solution 3

The common idiom used in overriding equals() is

@Override
public boolean equals(Object obj) {
       if (! (obj instanceof Product) ) return false;

       final Product other = (Product) obj;
       if (id != other.id)
           return false;
       return true;
   }

In the second version that you posted:

• the first if() may be good for optimization only if the following checks are too much expensive. But this is not the case, so that is just redundant code which is evil.
• That version won't work if you define a Product subclass which does not change the semantics of method equals(). (For example a class which provides some convenience method but no additional internal state to the objects.) This is because of the third if().