On Sun, 1 Jan 2023 15:23:16 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:

>> 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).

In my opinion that's fine. The iterator might not use its reference to the 
list, but it's using its type information.

As for creating a new iterator on every invocation (like empty set does) or 
returning the same instance (like empty list does), I don't mind that much and 
I can't imagine it mattering in terms of performance.

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

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

Reply via email to