RFR: 8320796: CssMetaData.initStyleables() [v2]

Kevin Rushforth kcr at openjdk.org
Tue Nov 28 00:09:18 UTC 2023


On Mon, 27 Nov 2023 21:39:31 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

>> Provides a public utility method for use by the skins (core and custom) to simplify initialization of styleable properties.
>> 
>> 
>> + /**
>> + * Utility method which combines CssMetaData items in one unmodifiable list with the size equal to the number
>> + * of items it holds (i.e. with no unnecessary overhead).
>> + *
>> + * @param list the css metadata items, usually from the parent, not nullable
>> + * @param items the additional items
>> + * @return the unmodifiable list containing all of the items
>> + *
>> + * @since 22
>> + */
>> + public static List<CssMetaData<? extends Styleable, ?>> initStyleables(
>> + List<CssMetaData<? extends Styleable, ?>> list,
>> + CssMetaData<? extends Styleable, ?>... items)
>
> Andy Goryachev has updated the pull request incrementally with one additional commit since the last revision:
> 
>   unmodifiable array list

This seems like a useful utility method. I left a few API doc comments.

modules/javafx.graphics/src/main/java/javafx/css/CssMetaData.java line 332:

> 330: 
> 331:     /**
> 332:      * Utility method which combines CssMetaData items in one unmodifiable list with the size equal to the number

I recommend using code format for `CssMetaData`

modules/javafx.graphics/src/main/java/javafx/css/CssMetaData.java line 333:

> 331:     /**
> 332:      * Utility method which combines CssMetaData items in one unmodifiable list with the size equal to the number
> 333:      * of items it holds (i.e. with no unnecessary overhead).

I recommend spelling out the abbreviation `i.e.`, maybe something like `in other words,` or `thus`.  You might even consider breaking this into two sentences. The "with size equal to" part is probably a more detailed description than is needed for the first sentence summary that shows up in the summary list of methods.

modules/javafx.graphics/src/main/java/javafx/css/CssMetaData.java line 335:

> 333:      * of items it holds (i.e. with no unnecessary overhead).
> 334:      *
> 335:      * @param list the css metadata items, usually from the parent, not nullable

I know what you mean, but this would be the first use of "nullable" in our docs. We usually just say "must not be null".

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

PR Review: https://git.openjdk.org/jfx/pull/1296#pullrequestreview-1751631072
PR Review Comment: https://git.openjdk.org/jfx/pull/1296#discussion_r1406908561
PR Review Comment: https://git.openjdk.org/jfx/pull/1296#discussion_r1406909831
PR Review Comment: https://git.openjdk.org/jfx/pull/1296#discussion_r1406906902


More information about the openjfx-dev mailing list