On Fri, 4 Aug 2023 15:01:51 GMT, Pavel Rappo <pra...@openjdk.org> wrote:

>> src/java.base/share/classes/sun/security/x509/PolicyInformation.java line 
>> 140:
>> 
>>> 138:     @Override
>>> 139:     public int hashCode() {
>>> 140:         return Objects.hash(policyIdentifier, policyQualifiers);
>> 
>> What is the criteria for changing the hashCode() impl? It seems that you 
>> change some and not others. With the many hashCode() impls (e.g. 
>> Objects.hash(), Arrays.hashCode()), I wonder why this and not that.
>
> Whether I suggest changing hashCode or not depends on multiple criteria, of 
> which these two are the most important:
> 
>   * Is the value specified or relied upon?
>   * Does it need to be performant?
> 
> If the answer to both of these is "NO", I might refactor hashCode. One 
> unintended side effect, is that sometimes such a refactoring improves 
> hashCode quality (think: perfect hashing) almost for free.
> 
> If you have concerns on particular refactorings, I'd be happy to discuss them.

I would add to that list that I think it is really important any change to the 
`hashCode` impl doesn't break the equals/hashCode contract, so it's important 
to look at the `equals` implementation in conjunction with any changes to 
`hashCode`, mostly to ensure 2 equal objects continue to have the same hash 
code. It is also important to question any oddity about the `hashCode` impl, 
like I did for DESKey and try to understand why it may have been implemented 
that way, such as skipping the first byte.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/14738#discussion_r1284801790

Reply via email to