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