RFR: JDK-8298528: Clean up raw type warnings in base in bindings and collections packages

John Hendrikx jhendrikx at openjdk.org
Sun Jan 1 15:27:53 UTC 2023


On Fri, 30 Dec 2022 20:53:01 GMT, Nir Lisker <nlisker at 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


More information about the openjfx-dev mailing list