RFR: 8290310: ChangeListener events are incorrect or misleading when a nested change occurs [v11]

[John Hendrikx]

On Sun, 9 Mar 2025 22:27:00 GMT, Nir Lisker <nlisker at 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