On Mon, 29 Mar 2021 15:17:22 GMT, Jeanette Winzenburg <faste...@openjdk.org> 
wrote:

>> modules/javafx.controls/src/main/java/javafx/scene/control/SkinBase.java 
>> line 250:
>> 
>>> 248:      *
>>> 249:      * @param observable the observable to observe for invalidation 
>>> events
>>> 250:      * @param consumer the consumer
>> 
>> Could be just me because I'm unfamiliar with this API, but I'd like to see 
>> an explanation for what the consumer is used for. If the consumer is 
>> executed on invalidation events on this observable, then clearly state it. 
>> Maybe the doc could be something like:
>> 
>>     Registers an action to take when an {@code Observable} is invalidated. 
>>     An {@code InvalidationListener} is registered on the given {@code 
>> observable} and the {@code consumer} is run 
>>     whenever the listener sends an invalidation event. If multiple consumers 
>> are registered on the same observable, 
>>     they are run in the order in which they were registered.
>> 
>>     @param observable the observable to observe for invalidation events
>>     @param consumer the action to take when the observable is invalidated
>> 
>> Since it's a `protected final` method, I don't think that the "Subclasses 
>> can use this..." portion is necessary - that's the only use of these methods.
>
> Changed the api doc along your suggestions (for invalidationListener related 
> methods only - those for the other types of listener will be changed 
> analogously once the wording is agreed on :)
> 
> did deviate a bit from your suggestion in
> 
> - instead of _action to take_ I used _operation to perform_ because in ui 
> contexts, _action_ feels a bit loaded and the replacement is from the doc of 
> Consumer in its overview and method doc for accept
> - removed mentioning of InvalidationListener: while we do have it, the 
> when/how/how many are added is an implementation detail 
> 
> Also clarified the null handling: both parameters can be null, if so nothing 
> happens.
> 
> Still not quite clear, when to use code tags in the description (vs. plain 
> words) - [styling 
> doc](https://www.oracle.com/de/technical-resources/articles/java/javadoc-tool.html)
>  seems to advocate it for all class names, actual java doc (including javafx 
> doc) seems to use it inconsistently.

The changes look good.

We use code tags when we refer to terms found in code, such as class, method 
and argument names, and literal like `true`, `class` and `null`. The docs have 
not been consistent, I know, but wherever we touch we try to fix it along the 
way. It's just not worth to do fixes specifically for that.

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

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

Reply via email to