On Tue, 6 Apr 2021 14:16:03 GMT, Jeanette Winzenburg <faste...@openjdk.org> 
wrote:

>> I guess I didn't look closely enough. Mystery solved, though.
>
>> I haven't gone back and done a detailed review yet, but I like the overall 
>> changes. The one thing I did notice is that the language used to describe 
>> the return value of `unregisterInvalidationListeners` and 
>> `unregisterListChangeListeners` is different:
>> 
> 
> just a side-note: technically, both are different because neither the new 
> listChangeListener nor the old changeListener methods are updated yet (we are 
> still at the first bullet of the plan, lazy me will update all to the same as 
> the invalidationListener wording, once we agree on it :)
> 
> So what we are arguing right now is the difference between new (= 
> invalidationListener) and old (= listChangeListener == c&p from 
> changeListener) method doc.
> 
>> ```
>> unregisterInvalidationListeners:
>>      * @return a composed consumer that performs all removed operations, or
>>      *  {@code null} if none have been registered or the observable is 
>> {@code null}
>> ```
>> 
>> ```
>> unregisterListChangeListeners:
>>      * @return A single chained {@link Consumer} consisting of all {@link 
>> Consumer consumers} registered through
>>      *      {@link #registerListChangeListener(ObservableList, Consumer)}. 
>> If no consumers have been registered on this
>>      *      list, null will be returned.
>> ```
>> 
>> I find it confusing to say that the returned list "performs all removed 
>> operations". Some might interpret this to mean that is is somehow necessary 
>> to call the returned chained consumer as part of the removal, which isn't 
>> what you meant to say.
>> 
>> How about something like this for both newly added unregister methods?
>> 
>> ```
>>      * @return a composed consumer consisting of all previously registered 
>> consumers, or
>>      * {@code null} if none have been registered or the observable is {@code 
>> null}
>> ```
> 
> agree: the _performs_ might be misleading. And I like your suggestion, though 
> I modified it a bit to be consistent with the rest of the changed doc which 
> focuses on "operations" (a term taken from Consumer api): 
> 
>      * @return a composed consumer representing all previously registered 
> operations, or
>      *  {@code null} if none have been registered or the observable is {@code 
> null}

I like this version, using "registered operations".

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

PR: https://git.openjdk.java.net/jfx/pull/409

Reply via email to