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);
}

Well, you check if this is the same instance or if this is an instance ( and so if it is not null in the same occasion) and then compare the hash. This seems not a bad idea but I might miss something — AxelH, Mar 09 '17 at 08:12
Also make sure you really want to use `instanceof` and not `getClass()`- see http://stackoverflow.com/q/596462/2513200 for arguments to choose one or the other. It is very easy to accidentally break the symmetry when using `instanceof`. — Hulk, Mar 09 '17 at 11:26

score 5 · Answer 1 · answered Mar 09 '17 at 08:12

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.

Darshan Mehta · Answer 2 · 2017-03-09T11:33:18.827

1

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.

edited Mar 09 '17 at 11:33

answered Mar 09 '17 at 08:24

Darshan Mehta

30,102
11
68
102

...and this would be a perfectly legal implementation for `hashCode` (but a very bad one, of course). – Hulk Mar 09 '17 at 10:58

score 1 · Accepted Answer · edited May 23 '17 at 12:31

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.

score 0 · Answer 4 · answered Mar 09 '17 at 08:32

0

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.

answered Mar 09 '17 at 08:32

Lew Bloch

3,364
1
16
10

Is this equals legal?

4 Answers4

Linked