On Sun, 9 Mar 2025 22:27:00 GMT, Nir Lisker <nlis...@openjdk.org> wrote:
>> John Hendrikx has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Fix non-convergence logic one more time...
>
> modules/javafx.base/src/main/java/com/sun/javafx/binding/OldValueCachingListenerManager.java
> line 90:
>
>> 88: else {
>> 89: setData(instance, new OldValueCachingListenerList<>(data,
>> listener));
>> 90: }
>
> This can now be a pattern matching `switch`:
>
> switch (data) {
> case null -> setData(instance, listener);
> case OldValueCachingListenerList<?> list -> list.add(listener);
> case ChangeListenerWrapper<?> wrapper -> {
> var list = new
> OldValueCachingListenerList<>(wrapper.listener, listener);
>
> list.putLatestValue(wrapper.latestValue);
>
> setData(instance, list);
> }
> default -> setData(instance, new
> OldValueCachingListenerList<>(data, listener));
> }
>
> Note that in the case of `ChangeListenerWrapper` (in your code too) there's
> no compile-time need to cast to a `T` type because
> `OldValueCachingListenerList` takes `Object`s and so does `setData`. Is this
> a required runtime check?
This looks like a good change. The runtime check is not required, I just always
specify generics when needed, and in this case I thought I had no choice but to
use `<T>`. I've adjusted it slightly:
switch (getData(instance)) {
case null -> setData(instance, listener);
case OldValueCachingListenerList<?> list -> list.add(listener);
case ChangeListenerWrapper<?> wrapper -> {
OldValueCachingListenerList<Object> list = new
OldValueCachingListenerList<>(wrapper.listener, listener);
list.putLatestValue(wrapper.latestValue);
setData(instance, list);
}
case Object data -> setData(instance, new
OldValueCachingListenerList<>(data, listener));
}
Primarily, I've not declared a `data` local anymore (so you can't use it
accidentally in the cases). For this I removed the default case and instead
did `case Object data ->`. It's still exhaustive :)
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1081#discussion_r1986766329
