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

[Kevin Rushforth]

On Thu, 20 Apr 2023 08:28:20 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

>> 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.
>
> Yeah I'm aware they want to add this, but it doesn't really invalidate my point as that syntax is terrible still.  Method references are far preferred whenever possible.
> 
> I stand by my point that using the observable parameter is a rare and quite advanced use case, where these API's would be intended to make it easier and safer to use method references and lambda's.
> 
> When dealing with multiple listeners that require the `Observable`, you are likely also dealing with adding and removing that single listener in some dynamic fashion.  Subscriptions don't work well with this as you'd need to track these, unlike `removeListener` which you can just pass the single listener reference.
> 
> Here's one of the only examples in JavaFX that uses the observable parameter (if you know of others, I'd be interested to see those as well):
> 
>         private final ChangeListener<Boolean> paneFocusListener = new ChangeListener<>() {
>             @Override public void changed(ObservableValue<? extends Boolean> observable, Boolean oldValue, Boolean newValue) {
>                 if (newValue) {
>                     final ReadOnlyBooleanProperty focusedProperty = (ReadOnlyBooleanProperty) observable;
>                     final TitledPane tp = (TitledPane) focusedProperty.getBean();
>                     focus(accordion.getPanes().indexOf(tp));
>                 }
>             }
>         };
> 
> And its management code:
> 
>         private final ListChangeListener<TitledPane> panesListener = c -> {
>             while (c.next()) {
>                 if (c.wasAdded()) {
>                     for (final TitledPane tp: c.getAddedSubList()) {
>                         tp.focusedProperty().addListener(paneFocusListener);
>                     }
>                 } else if (c.wasRemoved()) {
>                     for (final TitledPane tp: c.getRemoved()) {
>                         tp.focusedProperty().removeListener(paneFocusListener);
>                     }
>                 }
>             }
>         };
> 
> To do this with `Subscription::unsubscribe` you'd need to track the `Subscriptions`... something like:
> 
>         private final Map<Property<?>, Subscription> subscriptions = new HashMap<>();
>         private final ListChangeListener<TitledPane> panesListener = c -> {
>             while (c.next()) {
>                 if (c.wasAdded()) {
>                     for (final TitledPane tp: c.getAddedSubList()) {
>                         subscriptions.put(tp.focusedProperty(), tp.focusedProperty().changes(paneFocusListener));
>                   ...

I think the three newly added methods are a good choice. I wonder if we can some up with better names, though. without some verb like "add" or "subscribe" in the name, the name doesn't really indicate that it is adding a new listener to the observable.

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

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