RFR: JDK-8304439: Subscription based listeners [v2]
Kevin Rushforth
kcr at openjdk.org
Fri Jun 23 22:01:17 UTC 2023
On Fri, 16 Jun 2023 09:45:04 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:
>> I agree that the chosen names `invalidation`, `changes` and `values` are a bit terse. The whole signature (without reading docs) should make it clear you are creating a subscription, but perhaps we can do better. The use of `addListener` can be ruled out as it would conflict with the existing method due to having Lambda's with the same arity (the `values` listener would conflict with `addListener(InvalidationListener)`. Also, an `add` method would probably have users expecting a corresponding `remove` method.
>>
>> A few ideas listed here:
>>
>> | invalidation | values | changes |
>> |---|---|---|
>> |`subscribe(Runnable)`(*)|`subscribe(Consumer)`(*)|`subscribe(BiConsumer)`(*)|
>> |`subscribeInvalidations(Runnable)`|`subscribeValues(Consumer)`|`subscribeChanges(BiConsumer)`|
>> |`invalidationsTo(Runnable)`|`valuesTo(Consumer)`|`changesTo(BiConsumer)`|
>>
>> (*) May limit future listener types that have same arity, but can still be a good choice
>
> On that same topic of naming methods:
>
> What do people think of `Subscription#unsubscribe`? Should it be `cancel`? Something else? Okay as it is?
>
> Code example:
>
> if (subscription != null) {
> subscription.unsubscribe();
> subscription = null;
> }
> | invalidation | values | changes |
> |---|---|---|
> |`subscribe(Runnable)`(*)|`subscribe(Consumer)`(*)|`subscribe(BiConsumer)`(*)|
> |`subscribeInvalidations(Runnable)`|`subscribeValues(Consumer)`|`subscribeChanges(BiConsumer)`|
> |`invalidationsTo(Runnable)`|`valuesTo(Consumer)`|`changesTo(BiConsumer)`|
>
> (*) May limit future listener types that have same arity, but can still be a good choice
My preference is in the order you listed them. If we go with `subscribe`, using method name overloading, do want to add a future overload that takes a type with the same arity as one of the existing three, we can always assign a new name to that new method (since adding another overload wouldn't be source compatible, we would likely need a new name at that point).
@nlisker @andy-goryachev-oracle @mstr2 - what do you think?
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1069#discussion_r1240440189
More information about the openjfx-dev
mailing list