Common equality pitfalls while overriding equals and hashcode

Common pitfalls that can cause inconsistent behavior when overriding equals

Pitfall #1: Defining equals with the wrong signature

public class Point {

  private final int x;

  private final int y;

  public Point(int x, int y) {

    this.x = x;

    this.y = y;

  }

  public int getX() {

    return x;

  }

  public int getY() {

    return y;

  }

  // ...

}

A seemingly obvious, but wrong way would be to define it like this :

// An utterly wrong definition of equals

public boolean equals(Point other) {

  return (this.getX() == other.getX() && this.getY() == other.getY());

}

What's wrong with this method? At first glance, it seems to work OK :

Point p1 = new Point(1, 2);

Point p2 = new Point(1, 2);

Point q = new Point(2, 3);

System.out.println(p1.equals(p2));  // prints true

System.out.println(p1.equals(q));  // prints false

However, trouble starts once you start putting points into a collection :

import java.util.HashSet;

HashSet coll = new HashSet();

coll.add(p1);

System.out.println(coll.contains(p2)); // prints false

How can it be that coll does not contain p2, even though p1 was added to it, and p1 and p2 are equal objects ?

The reason becomes clear in the following interaction, where the precise type of one of the compared points is masked.

Define p2a as an alias of p2, but with type Object instead of Point :

Object p2a = p2;

Now, were you to repeat the first comparison, but with the alias p2a instead of p2, you would get:

System.out.println(p1.equals(p2a)); // prints false

What went wrong ?

In fact, the version of equals given previously does not override the standard method equals, because its type is different.

Here is the type of the equals method as it is defined in the root class Object:

public boolean equals(Object other)

Because the equals method in Point takes a Point instead of an Object as an argument, it does not override equals in Object.

Instead, it is just an overloaded alternative.

Overloading in Java is resolved by the static type of the argument, not the run-time type. So as long as the static type of the argument is Point, the equals method in Point is called. However, once the static argument is of type Object, the equals method in Object is called instead. This method has not been overridden, so it is still implemented by comparing object (not the contents).

That's why the comparison "p1.equals(p2a)" yields false even though points p1 and p2a have the same x and y values. That's also why the contains method in HashSet returned false. Since that method operates on generic sets, it calls the generic equals method in Object instead of the overloaded variant in Point.

A better equals method is the following : 

// A better definition, but still not perfect

@Override

public boolean equals(Object other) {

  boolean result = false;

  if (other instanceof Point) {

    Point that = (Point) other;

    result = 

        (this.getX() == that.getX() && this.getY() == that.getY());

  }

  return result;

}

Now,  equals has the correct type. It takes a value of type Object as parameter and it yields a boolean result. The implementation of this method uses instanceof and a cast.

It first tests whether the other object is also of type Point. If it is, it compares the coordinates of the two points and returns the result. Otherwise the result is false.

Pitfall #2: Changing equals without also changing hashCode

If you repeat the comparison of p1 and p2a with the latest definition of Point defined previously, you will get true, as expected. However, if you repeat the HashSet.contains test, you will probably still get false : 

Point p1 = new Point(1, 2);

Point p2 = new Point(1, 2);

HashSet coll = new HashSet();

coll.add(p1);

System.out.println(coll.contains(p2)); // prints false (probably)

In fact, this outcome is not 100% certain. You might also get true from the experiment. If you do, you can try with some other points with coordinates 1 and 2. Eventually, you'll get one which is not contained in the set. What goes wrong here is that Point redefined equals without also redefining hashCode.

Note that the collection in the example above is a HashSet. This means elements of the collection are put in "hash buckets" determined by their hash code. The contains test first determines a hash bucket to look in and then compares the given elements with all elements in that bucket. Now, the last version of class Point did redefine equals, but it did not at the same time redefine hashCode. So hashCode is still what it was in its version in class Object: some transformation of the address of the allocated object. The hash codes of p1 and p2 are almost certainly different, even though the fields of both points are the same.

Different hash codes mean with high probability different hash buckets in the set.

The contains test will look for a matching element in the bucket which corresponds to p2's hash code.

In most cases, point p1 will be in another bucket, so it will never be found. p1 and p2 might also end up by chance in the same hash bucket. In that case the test would return true.

The problem was that the last implementation of Point violated the contract on hashCode as defined for class Object:

If two objects are equal according to the equals(Object) method, then calling the hashCode method on each of the two objects must produce the same integer result.

In fact, it's well known in Java that hashCode and equals should always be redefined together.

Furthermore, hashCode may only depend on fields that equals depends on.

For the Point class, the following would be a suitable definition of hashCode : 

public class Point {

  private final int x;

  private final int y;

  public Point(int x, int y) {

    this.x = x;

    this.y = y;

  }

  public int getX() {

    return x;

  }

  public int getY() {

    return y;

  }

  @Override

  public boolean equals(Object other) {

    boolean result = false;

    if (other instanceof Point) {

      Point that = (Point) other;

      result = 

         (this.getX() == that.getX() && this.getY() == that.getY());

   }

   return result;

  }

 @Override

  public int hashCode() {

   return (41 * (41 + getX()) + getY());

  }

}

This is just one of many possible implementations of hashCode. Adding the constant 41 to one integer field x, multiplying the result with the prime number 41, and adding to that result the other integer field y gives a reasonable distribution of hash codes at a low cost in running time and code size.

Adding hashCode fixes the problems of equality when defining classes like Point.