On Fri, 30 Dec 2022 20:53:01 GMT, Nir Lisker <nlis...@openjdk.org> 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