Community
Participate
Working Groups
Generating hashCode and equals with the new feature "Use Objects.hash and Objects.equals method" considers super.equals, but not super.hashCode. When using the old style, super.hashCode will be considered. Current generated hashCode method: public int hashCode() { return Objects.hash(variable1, ...) } Suggested hashCode method: public int hashCode() { return Objects.hash(super.hashCode(), variable1, ...) }
@Pierre-Yves, can you please take care of this?
Of course this should only be done if necessary ( check GenerateHashCodeEqualsOperation.needsNoSuperCall as for the old code) But maybe it should better be return 31 * super.hashCode() + Objects.hash(variable1, ...) to avoid autoboxing of the super.hashCode() result
Sure, I'll look into it. Thanks for reporting the issue @Lars!
I've spotted another issue when inner classes are used. Having several "if (!fUseJ7HashEquals) body.statements().add(...)" in the implementation is confusing and is partly what put us in trouble with these two bugs. I'll strengthen the test suite, perform some refactoring and fix these issues.
(In reply to Till Brychcy from comment #2) > But maybe it should better be > return 31 * super.hashCode() + Objects.hash(variable1, ...) > to avoid autoboxing of the super.hashCode() result Wondering some more about this comment. For primitive types, we currently have autoboxing behaviour with the hashCode implementation, i.e. Objects.hash(aBool, aByte, aChar, anInt, aFloat). Is this something we want to address? I've come up with two implementation variants, one which simply fixes the issues we've picked up so far in this bug report (super call and inner class), the other one with slightly cleaner refactoring which also happens to address autoboxing for fields. I'm in two minds here. Even though Objects.hash(aBool, aByte, aChar, anInt, aFloat) causes autoboxing, it seems significantly more succinct than what a non autoboxing implementation would generate: final int prime = 31; int result = 1; result = prime * result + (aBool ? 1231 : 1237); result = prime * result + aByte; result = prime * result + aChar; result = prime * result + anInt; result = prime * result + Float.floatToIntBits(aFloat); return result; What do you think?
Autoboxing is a concern, but readability is much better with one single Objects.hash. If there are performance issues with the autoboxed variant, one could always switch back to the old style. Taking a look at the future of java, autoboxing may not be an issue anymore after implementing value types/JEP 169 (http://openjdk.java.net/jeps/169).
(In reply to Lars Briem from comment #6) > Autoboxing is a concern, but readability is much better with one single > Objects.hash. If there are performance issues with the autoboxed variant, > one could always switch back to the old style. I agree. Only I wonder if it even looks better to have the super.hashCode() inside the Object.hashCode(...) invocation. I think not, so I'd go for the version without auto boxing just for this case, not for the fields. But I don't have a strong opinion. > > Taking a look at the future of java, autoboxing may not be an issue anymore > after implementing value types/JEP 169 (http://openjdk.java.net/jeps/169). I doubt JEP 169 will help in situations like here (where a method expects an Object array of mixed types)
@Noopur, adding you to CC in case you have an opinion on the topic
New Gerrit change created: https://git.eclipse.org/r/130529
Gerrit change https://git.eclipse.org/r/130529 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=bbe552787fd049eb492c05ec98276ed5b528a768
(In reply to Eclipse Genie from comment #9) > New Gerrit change created: https://git.eclipse.org/r/130529 Thanks! During testing I noted two things that could be improved (also in the old implementation, so not related to this bug), for which we could open separate bugs: - There is a check whether the super class has equals and hashcode implemented. I think the same should be done for the enclosing instance. - If there are no fields, but an enclosing instance and/or a superclass, hashcode and equals generation is currently disallowed. I think it should be allowed. (In reply to Eclipse Genie from comment #10) > Gerrit change https://git.eclipse.org/r/130529 was merged to [master]. > Commit: > http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/ > ?id=bbe552787fd049eb492c05ec98276ed5b528a768 Released for 4.10M1
Thanks for reviewing! I'm definitely happy to raise some improvement bugs and have a go at tackling them. I agree with both your observations.
*** Bug 540890 has been marked as a duplicate of this bug. ***
