RFR: JDK-8199216: Memory leak and quadratic layout time with nested nodes (hbox) and pseudo-class in style sheet [v2]

[Michael Strauß]

On Fri, 31 Mar 2023 18:07:08 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

>> modules/javafx.graphics/src/main/java/com/sun/javafx/css/BitSet.java line 584:
>> 
>>> 582:      * @param obj the object to cast, cannot be {@code null}
>>> 583:      * @return a type T, or {@code null} if the argument was not of this type
>>> 584:      * @throws NullPointerException when {@code obj} is {@code null}
>> 
>> Previously, this method always returned an instance of `T`. Now that is not the case, it might also simply return `null` if the argument passed into it is an instance of a different class. I think it makes sense to also return `null` when the argument passed into the method is `null`.
>
> I'm a bit unsure why that would be an improvement. Passing `null` to a function that doesn't expect it should IMHO never just return `null` but should instead be considered a programming error and result in a stack trace.  Passing in a non-null value that can't be casted is explicitly documented now that it would result in `null`.  One is a caller error, the other isn't IMHO (as the caller can't check if it is castable without another method -- I considered adding an `instanceof` method).
> 
> Or maybe I'm reading too much in to this and you are just pointing out that the function has changed from its previous contract -- I think this is okay as `BitSet` is not public API, nor are any of its subclasses.

I found it strange that a method named `cast` would reject `null` as an argument, but will happily return `null` and require the caller to check it.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1076#discussion_r1155395834