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

[John Hendrikx]

On Mon, 10 Apr 2023 19:18:42 GMT, Michael Strauß <mstrauss at openjdk.org> wrote:

>> John Hendrikx has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Move Subscription method for invalidations to Observable
>>   
>>   - moved Subscription class to the Observable package
>
> modules/javafx.base/src/main/java/javafx/beans/Observable.java line 107:
> 
>> 105:      * @since 21
>> 106:      */
>> 107:     default Subscription invalidations(Runnable subscriber) {
> 
> I think re-using `InvalidationListener` may fit better into the existing JavaFX API:
> 
> interface Observable {
>     Subscription subscribe(InvalidationListener);
> }
> 
> 
> The method can then be overloaded for `ChangeListener`, `ListChangeListener`, `SetChangeListener`, and `MapChangeListener` as well. Doing so allows users to use the same functional interfaces they've used before with the `addListener`/`removeListener` APIs.
> 
> As an added bonus, we don't need to allocate a wrapper for the listeners.

This would depend I think how you want to approach this.  I added three variants, invalidations, changes and values.  Using the same name `subscribe`, you need to make sure the lambda's used all have a different arity, or you'll need to add a cast to disambiguate them.

I purposely removed the first `Observable` parameter on all of the variants because I think it will be a much better match when you want to register a listener, and "pipe" it to some existing code.  Now we often have to "strip" the unnecessary parameter.  I can't do this easily for example:

      minimumWidthProperty().addListener(this::requestLayout);
      selectedItem.addListener(getSelectionModel()::select);
      modelStatus.addListener(statusLabel.textProperty()::set);

It must be these now:

      minimumWidthProperty().addListener(obs -> this.requestLayout());
      selectedItem.addListener((obs, o, n) -> gelectionModel().select(n));
      modelStatus.addListener((obs, o, n) -> statusLabel.setText(n));

The `values` variant is highly useful for this, as `ChangeListener` has a 2nd rarely used parameter (oldValue).

Also I think calling unsubscribe on subscriptions should preferably be the only way to remove a listener again; reusing the existing listeners would allow you to bypass this with `removeListener`.

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

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