On Fri, 31 Mar 2023 18:07:08 GMT, John Hendrikx <jhendr...@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
