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

[Nir Lisker]

On Mon, 27 Mar 2023 14:36:45 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

>> Makes `Subscription` public (removing some of its methods that are unnecessary), and adds methods that can provide `Subscription`s in `ObservableValue`.
>
> 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

Just took a quick look.

Ideally, we would not split the world into subscriptions and listeners. Adding a return type to `addListener` and `removeListener` is source compatible, but not binary. Is this something that we strictly can't do? Is the disruption factor too high?

modules/javafx.base/src/main/java/javafx/beans/Subscription.java line 51:

> 49:      * @throws NullPointerException when {@code subscriptions} is {@code null} or contains {@code null}
> 50:      */
> 51:     static Subscription of(Subscription... subscriptions) {

`combine` sounds like a better name to me.

modules/javafx.base/src/main/java/javafx/beans/Subscription.java line 58:

> 56:                 subscription.unsubscribe();
> 57:             }
> 58:         };

This can be
`return () -> list.forEach(Subscription::unsubscribe);`

modules/javafx.base/src/main/java/javafx/beans/Subscription.java line 85:

> 83:             other.unsubscribe();
> 84:         };
> 85:     }

This looks like a special case of the `of` method:

default Subscription and(Subscription other) {
   return of(this, other);
}

although this implementation creates an array, which might be what you're trying to avoid.

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

PR Review: https://git.openjdk.org/jfx/pull/1069#pullrequestreview-1389078204
PR Review Comment: https://git.openjdk.org/jfx/pull/1069#discussion_r1169356681
PR Review Comment: https://git.openjdk.org/jfx/pull/1069#discussion_r1169356850
PR Review Comment: https://git.openjdk.org/jfx/pull/1069#discussion_r1169356504