On Mon, 4 Mar 2024 16:12:12 GMT, Andy Goryachev <ango...@openjdk.org> 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

Reply via email to