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

[John Hendrikx]

On Mon, 10 Jul 2023 08:01:46 GMT, Jose Pereda <jpereda at openjdk.org> wrote:

>> I think it would be better to see value listeners as the listener alternative to what `bind` offers, except not limited to properties. The main purpose of these subscriptions is to keep things in sync, not so much to know when something changed. When you can use `bind` that would be ideal, but not all properties can be bound, and not everything you may want to keep in sync is a property.  `SelectionModel` is a good example; its properties can't be bound, but it does have methods to modify the selection:
>> 
>>      myModelSelection.subscribe(selectionModel::select);  // sync right away thanks to initial call
>> 
>> The same goes for other state that you may want to sync, like the example I gave in another reply:
>> 
>> 
>> node.sceneProperty()
>>   .flatMap(Scene::windowProperty)
>>   .flatMap(Window::showingProperty)
>>   .orElse(false)
>>   .subscribe(this::subscribeOrUnsubscribeDependingOnVisibility);
>> 
>> It's much nicer if this gets called immediately, to avoid having to duplicate this code to register the initial subscribers (and this code may need to check the visibility first still... was this UI part added to an existing visible UI?  Then register immediately, if it wasn't then do nothing and let the subscriber above handle it... )
>> 
>> I think calling subscribers immediately when it is possible and makes sense to do so should be the default.  For change subscribers it makes no sense as there is no change to report; for invalidation subscribers, you could again make the case that it may be good to call them immediately if the property isn't currently valid, if it weren't for the fact that `ExpressionHelper` will make a property always valid when you register an `InvalidationListener` (undocumented) -- and since it will always be valid immediately after subscription, there is no need to do that initial call.  (The `ExpressionHelper` making the property valid when a listener gets added may even have been a quick hack, as it will unnecessarily make something valid, which may result in **all** invalidation listeners getting called down the line, while the intention maybe was to only call the newly registered one if the property was currently invalid... too late to change this now though without breaking lots of code.
 )
>> 
>> So I realize this may seem inconsistent, but each subscriber type IMHO can stand on its own and can have a different purpose and different semantics, and I think that in the case of a value subscriber, there is a lot more benefit from callin...
>
> Thanks for the clarifications. 
> If I get it right, you talk about `invalidation subscriber` for `subscribe(Runnable)` API, `value subscriber` for `subscribe(Consumer)` API and `change subscriber` for `subscribe(BiConsumer)` API, and that's fine, but for a developer that approaches this new API, there is no indication of that (API naming, javadoc), so that's why I'm a little bit concerned about this different in behaviour of the value vs change subscribers.
> 
> As commented before, probably the javadoc for the three `subscribe` methods should be extended a little bit more with a mention of the listener involved, and with a small sample.

Yes, it's true they map like that: Runnable -> invalidations, Consumer -> values, BiConsumer -> changes.  I will extend the docs as this indeed is a bit unclear now (the methods were called "invalidations", "values" and "changes" before).

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

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