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

[Michael Strauß]

On Sun, 16 Apr 2023 12:34:44 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

>> I agree with you there, and I've been looking what would be a good way to achieve this.  I will take another look soon.  My primary concern is that this is a somewhat critical path, and I would want to ensure that it doesn't cause too much performance regressions (I've already been optimizing all of this code with the help of a JMH test)
>
> While looking that code over to see if it could be merged without impacting the general case, I discovered a small bug in the OldValueCaching version.  After I fixed it, the code was even more similar than it was already.  The only different still is the fact that the latest value must be kept track of whenever ObservableValue#getValue is called.
> 
> I've now added an extra parameter to the generic version to allow for storing the latest value when it is queried (and not storing it if it's not needed).  This seems to have a minimal performance impact only, so I think the trade off is acceptable.

Have you considered adding a method like `void valueUpdated(T value) {}` to `ListenerList`? This will require `ListenerList` to have a type variable `T` (which `OldValueCachingListenerList` adds anyway).

This method could then be called instead of `latestValueTracker.accept(newValue)`, and `OldValueCachingListenerList` can override it and store the value. The advantage of that would be that we don't need the `latestValueTracker` field.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1081#discussion_r1167979736