On Sun, 16 Apr 2023 17:20:08 GMT, Michael Strauß <[email protected]> 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
