Which part of the equals() general contract does my equals() not satisfy

Question

I'm fairly new to java and am just trying to get my head around understanding @Override of the equals() and hashcode() methods.
I know for the equals method to be correct it needs to be:

1. Reflexive: a.equals(a)
2. Symmetric: a.equals(b) then b.equals(a)
3. Transitive: a.equals(b) && b.equals(c) Then a.equals(c)
4. Not null: ! a.equals(null)

I am struggling to pinpoint which of the above properties I am and am not satisfying when writing my overide of the equals method.

I am aware that eclipse can generate these for me, however as I haven't yet gotten the concept fully, writing it out helps me to learn.

I have written out the what I think is the correct way to do it, but when I check with the eclipse generated version I seem to be 'missing' some aspects.

Example:

public class People {

    private Name first; //Invariants --> !Null, !=last
    private Name last;  // !Null, !=first
    private int age;    // !Null, ! <=0
    ...
}

What I wrote:

public boolean equals(Object obj){
    if (obj == null){
        return false;
    }
    if (!(obj instanceof People)){
         return false;
    }
    People other = (People) obj;
    if (this.age != other.age){
        return false;
    }
    if (! this.first.equals(other.first)){
        return false;
    }
    if (! this.last.equals(other.last)){
        return false;
    }
    return true;
}

vs eclipse generated

public boolean equals(Object obj) {
    if (this == obj)
        return true;
    if (obj == null)
        return false;
    if (getClass() != obj.getClass())
        return false;

    People other = (People) obj;
    if (first == null) {
        if (other.first != null)
            return false;
    } else if (!first.equals(other.first))
        return false; 
    if (age != other.age)
        return false;
    if (last == null) {
        if (other.last != null)
            return false;
    } else if (!last.equals(other.last))
        return false;

    return true;
}

I am missing:

• if (this == obj)
      return true;
  

• if (getClass() != obj.getClass())
      return false;
  

• And for each variable:

  if (first == null) {
      if (other.first != null)
          return false;
  } else if (!first.equals(other.first))
      return false;
  

I'm not sure what getClass() is and is my implmentation incorrect?

Answer 1

First piece of code:

if (this == obj)
    return true;

This improves performance in case you compare the object reference against itself. Example: a.equals(a);.

Second piece of code:

if (getClass() != obj.getClass())
    return false;

This compares if the class of the reference being compared is the same class of this. The difference between using this approach and instanceof is that it's more restrictive when comparing against a sub class. Example:

public class Foo { }
public class Bar extends Foo { }

//...
Foo foo = new Bar();
System.out.println(foo instanceof Bar); //prints true
System.out.println(foo instanceof Foo); //prints true
Foo foo2 = new Foo();
System.out.println(foo.getClass() == foo2.getClass()); //prints false

Which one should you choose? There's no good or bad approach, it will depend on your desired design.

Third piece of code:

if (first == null) {
        if (other.first != null)
            return false;
    } else if (!first.equals(other.first))
        return false; //For each variable.

This is simply a null check for each object reference field in the class. Note that if this.first is null then doing this.first.equals(...) will throw a NullPointerException.

Answer 2

I don't think your implementation is incorrect, but a few notes:

if (this == obj)
      return true;

Is a performance optimization, it directly tests for reference equality and short-circuits tests where a is a.

if (getClass() != obj.getClass())
        return false;

Is similar to your instanceof call, optimizes away a null check. The other calls seem to be null-checks.