On Mon, 4 Mar 2024 16:12:12 GMT, Andy Goryachev <[email protected]> wrote:
>> Karthik P K has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Add unit test
>
> modules/javafx.graphics/src/main/java/com/sun/javafx/scene/text/TextLayout.java
> line 108:
>
>> 106: if (obj == null)
>> 107: return false;
>> 108: if (getClass() != obj.getClass())
>
> any reason why this code uses getClass() != obj.getClass() ?
>
> perhaps a better choice might be the usual pattern
>
> if(x == this) {
> return true;
> } else if(x instanceof Hit h) {
> return charIndex == h.charIndex && ...
> }
> return false;
There are long discussions about this, and not really worth going into. They
both have their place. Generally, the `getClass` versions are easier to get
right without violating the `equals` contract. `instanceof` can be used, to
allow comparisons with (trivial) subtypes, but then you must make `equals`
final. Anything fancier violates the equals contract.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1323#discussion_r1511498972
