22

Possibly a dumb question, but I don't want to screw this up. Let's say I have two Java classes, Class1 and Class2, where Class2 extends Class1. I want to override Object.hashcode() using Guava for both classes. For the superclass, I've got

@Override
public int hashCode() {
    return Objects.hashcode(mField1, mField2);
}

For Class2, what's the right way to implement hashcode() that takes the members of Class1 into consideration? Is it like this?

@Override
public int hashcode() {
    return Objects.hashcode(super.hashcode(), mField3, mField4);
}  

That SEEMS right to me, but I'm looking for some validation. Joshua Bloch doesn't address this situation in Effective Java, and the Guava docs don't either.

Bozho
  • 588,226
  • 146
  • 1,060
  • 1,140
Andy Dennie
  • 6,012
  • 2
  • 32
  • 51
  • Perfectly reasonable implementation, if equality of instances of Class2 is predicated on the fields of Class1 as well - it would be a mistake (a performance bug to be specific) not to depend on super.hashCode(). – Dimitris Andreou Nov 27 '11 at 06:34
  • great answers and discussion, thanks to everybody. In my case, I don't need to test equality between instances of Class1 and Class2, but the points made below are good to know and understand. – Andy Dennie Nov 30 '11 at 13:53

3 Answers3

13

Yes, that looks correct. It would be the same if you had Objects.hashCode(f1, f2, f3, f4). If you look at the implementation, it's something like result += 31 * result + hashcodeOfCurrentObject. Which means that your result will be 31 + the super hashcode, which is not exactly the same, but would not be a problem.

Bozho
  • 588,226
  • 146
  • 1,060
  • 1,140
10

Effective Java does address this situation...by saying that you shouldn't do it. Item 8:

It turns out that this is a fundamental problem of equivalence relations in object-oriented languages. There is no way to extend an instantiable class and add a value component while preserving the equals contract, unless you are willing to forgo the benefits of object-oriented abstraction.

(Corollary: the same reasoning applies to hashCode().)

Louis Wasserman
  • 191,574
  • 25
  • 345
  • 413
  • No. hashCode() is not a binary relation, thus it does not need to be "symmetric", which is the property that screws equals() up for hierarchies. I see nothing in the question implying instances of Class1 can be equal to instances of Class2, which is the problem covered in E.J.. – Dimitris Andreou Nov 27 '11 at 06:29
  • It's not clear from the question either way, so I thought I'd at least try to cover this case. – Louis Wasserman Nov 28 '11 at 15:55
  • 2
    But to make it clearer: if x.equals(y), then x.hashCode() is contractually obligated to equal y.hashCode(). *If* we wish for instances of Class1 to be equal to instances of Class2, then we need for their hashCode() implementations to be consistent as well, at least in the cases where instances of the different classes are equal. – Louis Wasserman Nov 28 '11 at 16:17
  • Well, if you are not sure about the question, how are you sure about the answer? :-) I guess you could clarify by adding something like "the following answer is only applicable if you also have an equals() for which your hashCode() is broken". – Dimitris Andreou Nov 30 '11 at 04:44
  • 2
    Upvoting because, despite the concerns in these comments, in most cases people override `equals` and `hashcode` together, and so this point is very important. – bacar Jul 10 '12 at 15:24
  • 1
    ... and in the other cases, when people override `hashCode` but not `equals` (or `equals` but not `hashCode`) they are doing something bad. Or even immoral. – Piotr Findeisen Dec 22 '12 at 16:23
0

Although Bozho's suggestion is valid, I prefer this approach:

@Override
public int hashCode() {
    return Objects.hashcode(mField1, getParentField1(), getParentField2());
}
Sean Patrick Floyd
  • 292,901
  • 67
  • 465
  • 588
  • 2
    But then if the hashcode calculation of the parent class changes, the subclass's hashcode doesn't reflect the change. There's also a possibility that the superclass hashcode uses some private fields that the subclass doesn't have access to. – ColinD Nov 24 '11 at 15:16
  • @ColinD I know. This version only makes sense if you know and have access to the superclass. – Sean Patrick Floyd Nov 24 '11 at 16:47
  • I'm not sure I see the benefit of this approach; could you explain why you prefer it? The downside seems to be that now the subclass has to keep track of the fields of the superclass, which would be kind of a hassle, maintenance-wise. – Andy Dennie Nov 24 '11 at 17:12
  • 1
    @AndyD see Louis Wasserman's answer: Each class is responsible for its own contracts. – Sean Patrick Floyd Nov 24 '11 at 23:54