On Mon, 3 Jul 2023 11:12:32 GMT, Pavel Rappo <[email protected]> wrote:
> Please review this PR to use modern APIs and language features to simplify
> `equals` and `hashCode` in the java.text area.
>
> * Some changes to `equals` and `hashCode` are refactoring rather than
> modernization. Such changes can be as trivial as rearranging, adding, or
> commenting checks.
>
> * java.text area contains more classes whose `equals` and `hashCode` could be
> simplified; for example: sun.text.CompactByteArray or java.text.DigitList.
> However, I found no evidence of `equals` and `hashCode` in those classes
> being used in source or tests. Since writing new tests in this case seems
> unreasonable, I **excluded** those classes from this PR.
Marked as reviewed by rriggs (Reviewer).
src/java.base/share/classes/java/text/CompactNumberFormat.java line 2364:
> 2362: return true;
> 2363: }
> 2364:
Moving this before the `super.equals` call performance-wise would favor the
equals case. (statistics not available guess)
The code style in these files favors `if (this == obj)`... over the swapped
order of arguments.
src/java.base/share/classes/java/text/DecimalFormat.java line 2926:
> 2924: if (obj == this) { // disambiguate super equality from ref
> equality
> 2925: return true;
> 2926: }
Ditto comment in CompactNumberFormat.
src/java.base/share/classes/java/text/DecimalFormat.java line 2930:
> 2928: DecimalFormat other = (DecimalFormat) obj;
> 2929: return ((posPrefixPattern == other.posPrefixPattern &&
> 2930: positivePrefix.equals(other.positivePrefix))
This might also be:
return (obj instanceof DecimalFormat other) &&
posPrefixPattern == other.posPrefixPattern &&
positivePrefix.equals(other.positivePrefix));
-------------
PR Review: https://git.openjdk.org/jdk/pull/14752#pullrequestreview-1514810525
PR Review Comment: https://git.openjdk.org/jdk/pull/14752#discussion_r1253309018
PR Review Comment: https://git.openjdk.org/jdk/pull/14752#discussion_r1253310120
PR Review Comment: https://git.openjdk.org/jdk/pull/14752#discussion_r1253317064
