On Thu, 1 Apr 2021 15:03:27 GMT, Jeanette Winzenburg <[email protected]>
wrote:
>> modules/javafx.controls/src/main/java/javafx/scene/control/SkinBase.java
>> line 270:
>>
>>> 268: * may be {@code null}
>>> 269: * @return a composed consumer that performs all removed
>>> operations, or
>>> 270: * {@code null} if none have been registered or the observable is
>>> {@code null}
>>
>> Somehow my comment seems to have been deleted, so here it is again, slightly
>> modified, along with a suggestion.
>>
>> 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:
>>
>> 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}
>
> @Kevin
>
> don't worry, I have seen your comment (and been thinking about it :) - but
> also can't find it right now (nor can I find Nir's comment which started the
> overhaul ..), no idea what happened.
>
> Seeing your suggestion, it's similar to what keeps running in my mind.
Maybe GitHub decided to prune it. 😃
Btw, you tagged the wrong "Kevin" above.
-------------
PR: https://git.openjdk.java.net/jfx/pull/409
