23

Consider the following test case, is it a bad practice to use the hashCode method inside of equals as a convenient shortcut?

public class Test 
{    
    public static void main(String[] args){
        Test t1 = new Test(1, 2.0, 3, new Integer(4));
        Test t2 = new Test(1, 2.0, 3, new Integer(4));
        System.out.println(t1.hashCode() + "\r\n"+t2.hashCode());
        System.out.println("t1.equals(t2) ? "+ t1.equals(t2));
    }
    
    private int myInt;
    private double myDouble;
    private long myLong;
    private Integer myIntObj;
    
    public Test(int i, double d, long l, Integer intObj ){
        this.myInt = i;
        this.myDouble = d;
        this.myLong = l;
        this.myIntObj = intObj;
    }
    
    @Override
    public boolean equals(Object other)
    {        
        if(other == null) return false;
        if (getClass() != other.getClass()) return false;            
        
        return this.hashCode() == ((Test)other).hashCode();//Convenient shortcut?
    }

    @Override
    public int hashCode() {
        int hash = 3;
        hash = 53 * hash + this.myInt;
        hash = 53 * hash + (int) (Double.doubleToLongBits(this.myDouble) ^ (Double.doubleToLongBits(this.myDouble) >>> 32));
        hash = 53 * hash + (int) (this.myLong ^ (this.myLong >>> 32));
        hash = 53 * hash + (this.myIntObj != null ? this.myIntObj.hashCode() : 0);
        return hash;
    }   
}

Output from main method:

1097562307
1097562307
t1.equals(t2) ? true
Lii
  • 11,553
  • 8
  • 64
  • 88
bradvido
  • 2,743
  • 7
  • 32
  • 49

7 Answers7

45

In general, it's not at all safe to compare the hashCode instead of using equals. When equals returns false, hashCode may return the same value, per the contract of hashCode.

Lii
  • 11,553
  • 8
  • 64
  • 88
Ryan Stewart
  • 126,015
  • 21
  • 180
  • 199
  • Thanks, after reading the specification, I see that hashCode must return the same integer when equals() is true, but may also return the same integer when equals is false, which is why this shortcut is bad practice... "However, the programmer should be aware that producing distinct integer results for unequal objects may improve the performance of hashtables" – bradvido Sep 14 '11 at 14:11
  • What if the object is immutable and we use a crypto hash function (like SHA-256) to generate the hashcode of the objects? That reduces the possibility of hash collisions right? There are some hash functions where no one has ever found a collision, so practically we can assume that if the hash-code is equal for two objects then the objects are equal. Theoretically the two objects may be different. – asdf Sep 14 '21 at 14:33
16

Very bad! HashCode equality does not mean that equals returns true. The contract is that two objects that are equal must have the same hashCode. But it DOES NOT state the two objects with the same HashCode must be equal.

John B
  • 32,493
  • 6
  • 77
  • 98
10

As all other answers state this is bad practice. However, one situation where you might want to refer to the hash code within the equals method is in cases where you have an immutable object and have cached the hash code previously. This allows you to perform a cheap inexact comparison before performing a full comparison. For example:

public class MyImmutable {
    private final String a;
    private final String b;
    private final int c;

    private int hashCode;

    public MyImmutable(String a, String b, int c) {
        this.a = a;
        this.b = b;
        this.c = c;
    }

    @Override
    public boolean equals(Object o) {
        if (this == o) return true;
        if (o == null || getClass() != o.getClass()) return false;

        MyImmutable that = (MyImmutable) o;

        // Compare cached hashCodes first before performing more expensive comparison.
        return hashCode == that.hashCode() && c == that.c && !(a != null ? !a.equals(that.a) : that.a != null) && !(b != null ? !b.equals(that.b) : that.b != null);
    }

    @Override
    public int hashCode() {
        if (hashCode == 0) {
            // hashCode not cached, or it was computed as 0 (we recalculate it in this case).
            hashCode = a != null ? a.hashCode() : 0;
            hashCode = 31 * hashCode + (b != null ? b.hashCode() : 0);
            hashCode = 31 * hashCode + c;
        }

        return hashCode;
    }
}
Adamski
  • 54,009
  • 15
  • 113
  • 152
  • Would also work (with some modification) for mutable objects that cache their hash-codes. Such objects require an additional `boolean` to record whether the cache is up to date. – Raedwald Sep 14 '11 at 14:19
  • @Raewald: That's a great point. You could actually avoid using a boolean and simply set the hashCode back to 0, the trade-off being that if the hash code is legitimately 0 you compute it every time. – Adamski Sep 14 '11 at 14:21
  • That won't work as you've written it, will it? "That" will always have its hashcode calculated, but "this" will not. (As a side issue, in general there's no guarantee that calculating hashcodes is quicker than comparing for equality -- e.g. java.lang.String). – Paul Cager Sep 14 '11 at 14:53
  • "there's no guarantee that calculating hashcodes is quicker than comparing for equality": yes, `equals()` should first be checking whether both objects have a cahced hash-code, rather than calling `hashCode()`. – Raedwald Sep 14 '11 at 15:41
  • 2
    @Paul: Actually "that" will only have its hash code calculated if required because the hashCode() implementation will calculate and cache the hash code. You're also correct about there being no guarantee. However, the solution is aimed at situations where you're likely to be comparing against the same immutable objects multiple times (e.g. if you store them in a HashMap). In this situation the int-based hash code comparison offers an efficiency gain. – Adamski Sep 26 '11 at 11:29
  • I +1ed only because I couldn't +5! I was about to reply to state something similar, but then I found your answer already did (and very nicely) – Unai Vivi May 14 '12 at 08:34
  • 1
    I'd suggest changing the hashcode computation so that no object's hashcode will ever be zero. For example, one could add `if (hashCode==0) hashCode == c | 1;` That would cause a slight bias in hashcode values, but that should pose far less of a problem than would having some objects' hashcodes get continuously recalculated. – supercat Jan 21 '13 at 21:10
6

This is not OK. Hashcodes, by their very nature, are not guaranteed to be unique.

Ernest Friedman-Hill
  • 80,601
  • 10
  • 150
  • 186
5

Bad practice? More than that, it's completely wrong. Two un-equal objects can return the same hash code. Do not do this.

Sean Owen
  • 66,182
  • 23
  • 141
  • 173
3

Here is the contract, copied from the Object specification [JavaSE6]:

It is not required that if two objects are unequal according to the equals(Ob- ject) method, then calling the hashCode method on each of the two objects must produce distinct integer results. However, the programmer should be aware that producing distinct integer results for unequal objects may improve the performance of hash tables.

To answer your question, No. Not a good idea.

Kal
  • 24,724
  • 7
  • 65
  • 65
3

As Ryan Stewart says, your code as written is faulty.

A situation in which it might be useful to use the hash-code of your objects in equals() is when your object caches the hash codes, and determining equality is portentially expensive. In that case you might use equality of the cached hash codes as one of the early necesssary-but-not-sufficient checks for equality, to return false fast for most pairs of objects that are not equals().

Community
  • 1
  • 1
Raedwald
  • 46,613
  • 43
  • 151
  • 237