On Sat, 10 Dec 2022 21:55:03 GMT, Nir Lisker <nlis...@openjdk.org> wrote:
>> 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 > > 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. NPE is probably because some underlying sets will throw NPE even for operations like `contains(null)` (the immutable variants from `List.of` and `Set.of` are bad offenders in this area). CCE maybe for sets that use comparators internally. I've adjusted it as you suggested, and same for map. ------------- PR: https://git.openjdk.org/jfx/pull/969