On Thu, 1 Apr 2021 15:14:15 GMT, Kevin Rushforth <k...@openjdk.org> wrote:
>> ahh .. looks like you marked the conversation that contained the original >> comment as resolved: >> https://github.com/openjdk/jfx/pull/409#discussion_r604387696 .. > > 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} ------------- PR: https://git.openjdk.java.net/jfx/pull/409
