Apache Commons hashCode builder inconsistent hashes

Question

I am using the hash code builder as an instance variable of a pojo.

public class Pojo {
private HashCodeBuilder hashBuilder = new HashCodeBuilder(); 
private int i;
public setI(int i) {this.i = i}

@Override
public int hashCode() {           
    hashBuilder.append(id);        
    return hashBuilder.toHashCode();
}
}

Now if I set the value of i to the same value twice, then my hashcode result will be different. Is this a bug in the implementation?

I understand that it is happening because the hash code builder keeps a running total. But should it not give the same hash for the same set of values?

Also, if I don't follow the above approach then I will end up initialising the same hash code builder within the hashcode method of my pojo thousands of times as follows:

...
@Override
public int hashCode() {   
    hashBuilder = new HashBuilder();        
    hashBuilder.append(id);        
    return hashBuilder.toHashCode();
}
...

Is there a way to reset this running total so that every time I call hashcode with the same set of values I get a consistent answer?

And why would resetting be more efficient then creating a new instance? You shouldn't be reusing it in the first place imho. It would even fail with the same id and calling `hashCode` twice in a row. — M. Deinum, Dec 12 '16 at 11:10
well, you create one pojo and then end up calling hashcode `X` number of times. You will end up with `X` hash code builder objects which are potentially going to be garbage collected but you never know when that will happen. So until that time your memory usage increase. So I think it is better to have just one pojo? — Syed Ali, Dec 12 '16 at 11:13
NO it isn't. Especially not with short lived objects, they come more or less for free. They are gc'ed quite quick . Looks like premature optimization instead of a real measured thing. — M. Deinum, Dec 12 '16 at 11:14
hmm right then I'll assume no side effects of calling it X number of times - thanks for the clarification — Syed Ali, Dec 12 '16 at 11:16

Jiri Tousek · Answer 1 · 2016-12-12T11:33:19.080

1

The way you're doing it you're appending another id each time you call hashCode(). Since append() is accepting a value (not field name!), the HashCodeBuilder has no way to know whether you appended id twice or whether you have another field that has the same value in it.

Create the HashCodeBuilder locally in the hashCode(). Don't assume it would affect performance in a significant way unless you have profiled the application and proved that it does (aka. avoid premature optimization).

If you indeed want to avoid needless creation of HashCodeBuilder on each hashCode(), you'll have to track changes in setters and set the hashCodeBuilder to null whenever a value changes through setter; then use it in hashCode() if needed. Then you'd have to make sure all field modification go through setters. Quite a lot of work and many things can go wrong, so you really want to avoid all this if possible.

edited Dec 12 '16 at 11:33

answered Dec 12 '16 at 11:14

Jiri Tousek

12,211
5
29
43

or simple solution would be to provide a reset method in the hash builder? – Syed Ali Dec 12 '16 at 11:22
Obviously the library authors didn't see any need for such method. Creating a throwaway instance is cheap in Java. Instance variables affect thread-safety. Common usage is to only keep the instance locally in `hashCode()`, and if you want to go for the hash code caching, you'd do better caching the resulting hash code than keeping the builder anyway. – Jiri Tousek Dec 12 '16 at 11:26
Update - I've just read through the builder implementation, threadlocal is used so thread safety would not be an issue. – Jiri Tousek Dec 12 '16 at 11:33

Apache Commons hashCode builder inconsistent hashes

1 Answers1