On Sun, 16 Apr 2023 17:20:08 GMT, Michael Strauß <mstra...@openjdk.org> wrote:
>> 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. Yeah, I'll test that out, it's better not to have that extra field in that class. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1081#discussion_r1167981446