On Fri, 30 Dec 2022 20:53:01 GMT, Nir Lisker <[email protected]> wrote:
>> Packages fixed:
>> - com.sun.javafx.binding
>> - com.sun.javafx.collections
>> - javafx.beans
>> - javafx.beans.binding
>> - javafx.collections
>> - javafx.collections.transformation
>
> modules/javafx.base/src/main/java/javafx/collections/FXCollections.java line
> 709:
>
>> 707: private static class EmptyObservableList<E> extends AbstractList<E>
>> implements ObservableList<E> {
>> 708:
>> 709: private static final ListIterator<Object> iterator = new
>> ListIterator<>() {
>
> Isn't a better fix is to not make this class static and use `<E>`? Other
> lists create a new iterator on each invocation, but this is effectively a
> singleton.
> In fact, `EmptyObservableList` doesn't need to be declared because it's
> called only in one place, so it can be "inlined" when declaring the
> `EMPTY_LIST` field. Maybe this is out of scope.
I considered all that, and don't mind changing it if we're in agreement.
The reason I didn't is that it would subtly change the iterator (it would have
an unnecessary reference to its containing class).
> modules/javafx.base/src/main/java/javafx/collections/FXCollections.java line
> 1640:
>
>> 1638: @Override
>> 1639: public Iterator<E> iterator() {
>> 1640: return new Iterator<>() {
>
> Here the empty `Set` creates a listener on invocation, unlike in the list
> case. Might want to keep a single pattern. I prefer the one with a singleton
> iterator because the empty set itself is a singleton. Same comment about
> considering "inlining" it.
Can make these consistent if the approach is agreed upon.
-------------
PR: https://git.openjdk.org/jfx/pull/972
