RFR: JDK-8304439: Subscription based listeners [v2]
Michael Strauß
mstrauss at openjdk.org
Tue Apr 18 00:57:52 UTC 2023
On Tue, 18 Apr 2023 00:36:22 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:
>> 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`.
These example would probably look like this in the near future:
``` java
minimumWidthProperty().addListener(this::requestLayout());
minimumWidthProperty().addListener(_ -> this.requestLayout());
selectedItem.addListener(getSelectionModel()::select);
selectedItem.addListener((_, _, value) -> gelectionModel().select(value));
modelStatus.addListener(statusLabel.textProperty()::set);
modelStatus.addListener((_, _, value) -> statusLabel.setText(value));
You can always ignore the `Observable` parameter, but you can't bring it back once it's gone. I have often used this parameter, for example when my listener handles changes of multiple similar properties. Removing functionality just to make it look a little prettier is not enough justification in my opinion.
If a subscription shouldn't be removable by passing the listener to `removeListener`, this can easily be achieved by internally wrapping it into another listener (just what you did, only with the functional interface being different). However, this might not be the best solution, as it doubles the number of listener objects when using the subscription API.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1069#discussion_r1169386664
More information about the openjfx-dev
mailing list