On Mon, 11 Jul 2022 22:44:41 GMT, Andy Goryachev <d...@openjdk.org> wrote:
>> - added missing hashCode() methods
>
> Andy Goryachev has updated the pull request incrementally with one additional
> commit since the last revision:
>
> 8289389: minimize the impact of collision
Except for one case, looks good. I think that nearly all of the `equals`
implementations are not the best (or just wrong), but it's outside the scope of
the PR.
modules/javafx.controls/src/test/java/test/javafx/scene/control/ListViewTest.java
line 1715:
> 1713: @Override public boolean equals(Object obj) {
> 1714: if (obj == null) return false;
> 1715: return id == ((RT22599_DataType)obj).id;
I note that the `equals` method here is wrong. The `hashCode` implementation is
fine.
modules/javafx.controls/src/test/java/test/javafx/scene/control/TableViewTest.java
line 5087:
> 5085: @Override public boolean equals(Object obj) {
> 5086: if (obj == null) return false;
> 5087: return id == ((RT22599_DataType)obj).id;
Same as `ListViewTest`.
modules/javafx.controls/src/test/java/test/javafx/scene/control/TreeTableViewTest.java
line 5743:
> 5741: @Override public boolean equals(Object obj) {
> 5742: if (obj == null) return false;
> 5743: return id == ((RT22599_DataType)obj).id;
Same as `ListViewTest`.
modules/javafx.controls/src/test/java/test/javafx/scene/control/TreeViewTest.java
line 3077:
> 3075: @Override public boolean equals(Object obj) {
> 3076: if (obj == null) return false;
> 3077: return id == ((RT22599_DataType)obj).id;
Same as `ListViewTest`.
modules/javafx.fxml/src/main/java/javafx/fxml/FXMLLoader.java line 2322:
> 2320: }
> 2321: return false;
> 2322: }
Same as `ListViewTest`.
modules/javafx.media/src/main/java/com/sun/media/jfxmediaimpl/NativeMediaAudioClipPlayer.java
line 457:
> 455: public int hashCode() {
> 456: return sourceClip.hashCode();
> 457: }
Why are you ignoring the significant fields? `equals` compares `priority`,
`loopCount`, `volume`, `balance`, `rate` and `pan` in addition.
modules/javafx.media/src/main/java/com/sun/media/jfxmediaimpl/NativeMediaAudioClipPlayer.java
line 531:
> 529: return 0;
> 530: }
> 531: return player.hashCode();
This is fine, but can also be a ternary `return player == null ? 0 :
player.hashCode();`
-------------
Changes requested by nlisker (Reviewer).
PR: https://git.openjdk.org/jfx/pull/821
