On Sat, 10 Dec 2022 08:23:07 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:

>> - Added generics (to package private or internal classes only)
>> - Minor clean-ups of code I touched (naming)
>> - Fixed incorrect use of generics
>> - Fixed raw type warnings
>> 
>> Note: some raw types have leaked into public API.  These could be fixed 
>> without incompatibilities.  For specifics see 
>> `JavaBeanObjectPropertyBuilder`.  The javadoc would have the method 
>> signatures change (`<T>` would be appended to the affected methods).  For 
>> now I've added a TODO there.
>
> John Hendrikx has updated the pull request incrementally with three 
> additional commits since the last revision:
> 
>  - Adjusted ReadOnlyListProperty#equals to be a bit more modern Java
>  - Remove SuppressWarnings in ReadOnlyListProperty
>  - Use assignment in instanceof

The generifying of the classes looks fine to me, and technically is a fix for 
the raw type warnings on the listeners, so I think it's fine to do it here.

modules/javafx.base/src/main/java/javafx/beans/property/ReadOnlySetProperty.java
 line 117:

> 115:         @SuppressWarnings("unchecked")
> 116:         Set<E> c = (Set<E>) obj;  // safe cast as elements are only 
> referenced
> 117: 

I suggest to change it similarly to list:


        if (obj == this) {
            return true;
        }

        if (!(obj instanceof Set<?> otherSet) || otherSet.size() != size()) {
            return false;
        }
        try {
            return containsAll(otherSet);
        } catch (ClassCastException | NullPointerException unused)   {
            return false;
        }


I find it odd that there is a need to catch these exceptions, but `AbstractSet` 
does so too.

Same for the `Map` variant.

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

Changes requested by nlisker (Reviewer).

PR: https://git.openjdk.org/jfx/pull/969

Reply via email to