RFR: JDK-8304439: Subscription based listeners [v7]

[Jose Pereda]

On Sun, 9 Jul 2023 20:24:09 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

>> There doesn't seem to be a way to do the same for the other overloads:
>> * The `Runnable` overload is specified to be invoked _when the value becomes invalid_, which isn't happening when a subscription is added. Eagerly invoking the runnable without a valid->invalid transition does not conform to the specification of an invalidation listener.
>> * The `BiConsumer` overload accepts two values: the old value and the new value. But we don't know the old value, we only know the current value. Using the current value as both old and new value does not conform to the specification of a change listener.
>
> The reasoning is that this would be the most logical and convenient way for these subscriptions to work.  I think mstr2 said it well, but I'll run them down as well:
> 
> Invalidation subscribers are called when the property actually becomes invalid; since it may currently be valid, calling the subscriber immediately would send the wrong signal.  A case could be made to call the subscriber immediately if it is currently invalid, but I think whatever we choose here, it should work the same way as the current `addListener(InvalidationListener)` -- I'm pretty sure this one doesn't call the listener immediately when the property is currently invalid (this is something of a gotcha as well for beginning users of `InvalidationListener`s).
> 
> Change subscribers require the old value; this is only temporarily available when there's an actual change in the property. Old values are not cached, and can't be cached as this may prevent garbage collection of whatever the old value references.
> 
> The value listeners are a new breed, and specifically set up for convenient use.  They're also the only ones that can send a sensible initial value, and since I'm pretty sure that's almost always what you'd want, this is the default for this type of subscription.

Okay, that makes sense for the InvalidationListener. 

However, for the ChangeListener, as I see it, this PR gives two options, over the same listener: you can care only about the newValue, or you can care about both oldValue and newValue, both from the observableValue. These are then passed to the consumer/biConsumer that defines the ChangeListener, but I don't see any reason why this consumer/biconsumer needs to get initialized with any value at all, does it?

Only bindings apply the initial value: when you bind two properties (unidirectionally), one property will get any future values of the other, starting from the initial value. But I don't see that happening to changeListeners: when you add a changeListener to a property, I don't see the action being invoked with the current value of the property at that moment, only when the property does change from the current value.

In fact, if you run this test:


    private final StringProperty value = new SimpleStringProperty("Initial");

    @Test
    void subscribeConsumerShouldCallSubscriberImmediatelyAndAfterEachChange() {
        value.addListener((obs, ov, nv) -> System.out.println("changeListener, nv: " + nv));
        value.subscribe(nv -> System.out.println("consumer, nv = " + nv));
        value.subscribe((ov, nv) -> System.out.println("biconsumer, nv = " + nv));
        value.set("A");
}


you'll get:


    consumer, nv = Initial
    changeListener, nv: A
    consumer, nv = A
    biconsumer, nv = A


It feels wrong to see the first printout to me.

My point is that I don't see why we need to call the consumer at all before there is any actual change in the property.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1069#discussion_r1257562515