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

[Kevin Rushforth]

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

I reviewed the proposal in the JBS enhancement and the proposed API in this PR. I think this would be a useful addition to JavaFX. I left a few questions and comments.

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

> 74:      * @return a combined {@code Subscription} which will cancel both when
> 75:      *     cancelled, never {@code null}
> 76:      * @throws NullPointerException when {@code other} is {@code null}

Maybe add a sentence that says that this is equivalent to `Subscription.combine(this, other)`?

modules/javafx.base/src/main/java/javafx/beans/value/ObservableValue.java line 339:

> 337:      * @since 21
> 338:      */
> 339:     default Subscription values(Consumer<? super T> subscriber) {

Same question about the names: we might want to consider a name that has "add" or "subscribe" in the name.

modules/javafx.base/src/test/java/test/javafx/beans/SubscriptionTest.java line 1:

> 1: package test.javafx.beans;

Needs a standard copyright header.

modules/javafx.base/src/test/java/test/javafx/beans/value/ObservableValueSubscriptionsTest.java line 1:

> 1: package test.javafx.beans.value;

Needs a standard copyright header.

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

PR Review: https://git.openjdk.org/jfx/pull/1069#pullrequestreview-1480154322
PR Review Comment: https://git.openjdk.org/jfx/pull/1069#discussion_r1230076228
PR Review Comment: https://git.openjdk.org/jfx/pull/1069#discussion_r1230078584
PR Review Comment: https://git.openjdk.org/jfx/pull/1069#discussion_r1230079004
PR Review Comment: https://git.openjdk.org/jfx/pull/1069#discussion_r1230079093