Override the equals method

Question

I'm currently revising for an exam. On a past paper there was the question,

Override the equals method in the following class. The method shall check for content equality of the whole state.

 class Employee
    {
        String firstName;
        int age;
    } 

[2 marks]

I did some fiddling for the right answer and have come up so far with this. Is there a simpler way to answer the question and is this right? Many thanks for help.

 public class Employee
    {

     int age;

    public boolean equals(Object obj)
    {
        if(this == obj)
            {
                return true; //Reference equality.
            }    
        if(!(obj instanceof Employee)) 
            {
                return false; // not the same type.
            }
        Employee other = (Employee) obj;
        return firstName == other.firstName;
        return age == other.age;
        }
    }

Answer 1

A couple of points:

1. You need to check if obj is null.
2. To compare String contents in Java, use equals(), i.e. firstName.equals(other.firstName). Check to see if firstName is null first.

Here's an improved implementation:

public boolean equals(Object obj)
{
  if(obj == null)
  {
    return false;
  }
  if(this == obj)
  {
    return true; //Reference equality.
  }    
  if(this.getClass() != obj.getClass())
  {
    return false;
  }
  Employee other = (Employee) obj;
  if(firstName == null)
  {
    if(other.firstName != null)
    {
      return false;
    }
  }
  else if(!firstName.equals(other.firstName))
  {
    return false;
  }
  return age == other.age;
}

EDIT: Updated type comparison to make equals() symmetric in accordance with @Mark Peters' answer.

Answer 2

Use

return (((this.firstName == null || other.firstName == null)
            && this.firstName == other.firstName)
       ||
       this.firstName.equals(other.firstName))
       && age == other.age;

This handles null cases too.

Answer 3

The String firstName should be compared with .equals(), NOT ==. The == compare is ok for the primitive int age field.

What if both firstNames are identical, yet age is unequal? Shouldn't this fail?

Something like return (firstName.equals(obj.firstName)) && (age == obj.age);

Of course, that doesn't consider the case when this.firstName is null, which would result in a NullPointerException being thrown.

Are the Employees considered equal if both have null firstNames? What if one is null and the other not? Assuming both must be null, or both must be String.equals(), you could use:

return ((null == firstName && null == obj.firstName) 
  || (null != firstName && firstName.equals(obj.firstName)))
  && (age == obj.age);

instead of your 2 return statements. The rest looks ok.

Answer 4

One thing to note, and I wouldn't imagine you would get docked for this in an exam...

It's usually poor practice to do an instanceof when the class isn't final. The reason for that is that equals() must be symmetric. Accepting subclasses (who might also implement equals with their own new aspects) could cause it to not be symmetric.

Example (I think the example is the same used in Effective Java 2ed):

class Point {
    protected int x, y;
    //equals method uses instanceof Point and checks x and y values are the same
}

class ColorPoint extends Point {
    protected Color color;
    //equals method checks that it's a ColorPoint, that super.equals is true, 
    //then checks the Color
}

new Point(1, 2).equals(new ColorPoint(1, 2, Color.red)); //true
new ColorPoint(1, 2, Color.red).equals(new Point(1, 2)); //false

It's a very subtle point that even most of the answerers here didn't take into account. But it's the reason that most generators of equals (such as the one in your favourite IDE) tend to do exact class comparison:

  if ( this.getClass() != other.getClass() ) {
     return false;
  }

When the equals method uses instanceof it's usually a good move to document that subclasses must follow the exact same specification.

Answer 5

Your last line, the age comparison, is unreachable; you shouldn't use == to compare Strings; and you need to account for null values.

Since the general movement seems to be towards laying it all out for you, here's Eclipse's implementation:

public class Employee {
    private final String firstName;
    private final int age;

    public Employee(final String firstName, final int age) {
        super();
        this.firstName = firstName;
        this.age = age;
    }

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

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

And a battery of tests:

import org.junit.Test;

public class EmployeeTest {
    @Test
    public void testEmployeeEquals() {
        final Employee nullNameEmp = new Employee(null, -1);
        final Employee empA1 = new Employee("a", 1);
        final Employee empA1Clone = new Employee("a", 1);
        final Employee empA2 = new Employee("a", 2);
        final Employee empB1 = new Employee("b", 1);
        final Employee empB2 = new Employee("b", 2);
        final Employee subEmp = new Employee("a", 1) {

        };

        assert !nullNameEmp.equals(empA1);
        assert !nullNameEmp.equals(empA1Clone);
        assert !nullNameEmp.equals(empA2);
        assert !nullNameEmp.equals(empB1);
        assert !nullNameEmp.equals(empB2);
        assert !nullNameEmp.equals(subEmp);
        assert !nullNameEmp.equals(null);

        assert !empA1.equals(nullNameEmp);
        assert empA1.equals(empA1Clone);
        assert !empA1.equals(empA2);
        assert !empA1.equals(empB1);
        assert !empA1.equals(empB2);
        assert !empA1.equals(subEmp);
        assert !empA1.equals(null);

        assert !empA2.equals(nullNameEmp);
        assert !empA2.equals(empA1);
        assert !nullNameEmp.equals(empA1Clone);
        assert !empA2.equals(empB1);
        assert !empA2.equals(empB2);
        assert !empA2.equals(subEmp);
        assert !empA2.equals(null);

        assert !empB1.equals(nullNameEmp);
        assert !empB1.equals(empA1);
        assert !empB1.equals(empA1Clone);
        assert !empB1.equals(empA2);
        assert !empB1.equals(empB2);
        assert !empB1.equals(subEmp);
        assert !empB1.equals(null);

        assert !empB2.equals(nullNameEmp);
        assert !empB2.equals(empA1);
        assert !empB2.equals(empA1Clone);
        assert !empB2.equals(empA2);
        assert !empB2.equals(empB1);
        assert !empB2.equals(subEmp);
        assert !empB2.equals(null);

        assert !subEmp.equals(nullNameEmp);
        assert !subEmp.equals(empA1);
        assert !subEmp.equals(empA1Clone);
        assert !subEmp.equals(empA2);
        assert !subEmp.equals(empB1);
        assert !subEmp.equals(empB2);
        assert !subEmp.equals(null);

        assert nullNameEmp.equals(nullNameEmp);
        assert empA1.equals(empA1);
        assert empA1Clone.equals(empA1Clone);
        assert empA2.equals(empA2);
        assert empB1.equals(empB1);
        assert empB2.equals(empB2);
        assert subEmp.equals(subEmp);
    }
}

Answer 6

public boolean equals(Object o){
   if(this==o){ //same instance, no need to check more
       return true;
   }
   if(o instanceof Employee){ //when null this will yield false
       Employee other = (Employee) o;
       return (this.name == other.name || (this.name != null && this.name.equals(other.name)) && this.age == other.age;
   }
   return false;
}

Answer 7

To put together in one answer all the parts already mentioned:

public boolean equals(Object obj) {
    if(this == obj) {
        return true; //Reference equality.
    }
    if(obj == null || !(obj instanceof Employee))
    {
        return false; // not the same type.
    }
Employee other = (Employee) obj;
return (firstName.equals(other.firstName) && age == other.age);
}