On Fri, 16 Jun 2023 09:42:45 GMT, John Hendrikx <[email protected]> wrote:
>> 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.
>
> 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;
}
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1069#discussion_r1232031343
