Is this equals legal?

Question

I've came across this piece of code. I have never seen an equals implemented in such a way. What strike me was that it is really "neat", in the sense that is only takes one line of boilerplate.

However, the fact I have never seen this approach before make me suspicious. According to the contract of Java equals and hashCode, is the following implementation legal?

@Override
public boolean equals(Object o)
{
  return this == o || o instanceof DetailsPageTrackingRequest 
  && this.hashCode() == o.hashCode();
}

@Override
public int hashCode()
{
  //java.util.Objects
  return Objects.hash(pageTrackingRequest, searchId, productId);
}

Answer 1

This approach is wrong. Hash code equality is not conclusive because non-equal objects may have the same hash. It's an engineered-in bug.

Answer 2

It is probably a bad idea for the reasons already stated in other answers.

As for the "legal" aspects, the Contract of Object.equals states

The equals method implements an equivalence relation on non-null object references:

• It is reflexive: for any non-null reference value x, x.equals(x) should return true.
• It is symmetric: for any non-null reference values x and y, x.equals(y) should return true if and only if y.equals(x) returns true.
• It is transitive: for any non-null reference values x, y, and z, if x.equals(y) returns true and y.equals(z) returns true, then x.equals(z) should return true.
• It is consistent: for any non-null reference values x and y, multiple invocations of x.equals(y) consistently return true or consistently return false, provided no information used in equals comparisons on the objects is modified.
• For any non-null reference value x, x.equals(null) should return false.

Step by step:

• reflexive: Yes, due to this == o
• symmetric: due to the use of instanceof, we would need to look at all super- and subclasses to be sure
• transitive: depends on the symmetry-requirement, otherwise Yes
• consistent: Yes
• x.equals(null) should return false: Yes, due to instanceof

So from a purely legal point of view, it depends on whether other implementations accross your inheritance-hierarchy violate the symmetry and transitivity - see the answers to Any reason to prefer getClass() over instanceof when generating .equals()?.

But aside from that, given the fact that hashCode is not required to produce different values for non-equivalent instances, it is usually not a good way to define equality.

---

An example:

A immutable point class with two fields x and y

class Point {
    final int x;
    final int y

    public Point(int x, int y) {
        this.x = x;
        this.y = y;
    }
}

-> there are 2^32 * 2^32 = 2^64 distinct states, but only 2^32 possible hashcodes. This means that there are lots of points that would be considered equal according to your implementation of equals.

---

Also see this example equals and hashCode: Is Objects.hash method broken? for someone stumbling over a hash-collision for hashes created with Objects.hash with Strings.

Answer 3

You should check the property values individually rather than just checking the hashCode. There is a chance that two completely different objects result in same hashCode that are not equal.

Also, equals should not solely rely on hashCode, e.g. if hashCode method gets changed to the following:

@Override
public int hashCode()
{
    return 31;
}

equals method would start returning true for all the objects which should not be the case.

Answer 4

It's up to you do define the criteria according to which two instances of a class are considered equal to each other.

However, you should consider that different combinations of the pageTrackingRequest, searchId & productId properties might result in the same hash code, and you might not want to consider such different combinations as equal to each other. It might make more sense for equals to require that all 3 properties are equal to each other respectively.