RFR: 8320796: CssMetaData.combine() [v4]

[John Hendrikx]

On Tue, 28 Nov 2023 00:51:36 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:
> 
>   combine

Marked as reviewed by jhendrikx (Committer).

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

> 330: 
> 331:     /**
> 332:      * Utility method which combines {@code CssMetaData} items in one unmodifiable list with size equal

Did you mean `immutable`?  As far as I know, the meaning of `unmodifiable` (in the Java world) means that the caller can't modify it, but the  provider still can.  For example:

      List<String> list = new ArrayList<>();
      List<String> unmodifiableList = Collections.unmodifiableList(list);
      list.add("A");  // both lists changed

Quote from collections docs:

> Note that changes to the backing collection might still be possible, and if they occur, they are visible through the unmodifiable view. Thus, an unmodifiable view collection is not necessarily immutable. However, if the backing collection of an unmodifiable view is effectively immutable, or if the only reference to the backing collection is through an unmodifiable view, the view can be considered effectively immutable.

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

> 342:         List<CssMetaData<? extends Styleable, ?>> list,
> 343:         CssMetaData<? extends Styleable, ?>... items)
> 344:     {

The method is not actually making use of `CssMetaData`, so a more generic utility method would have worked as well:

         public static <T> List<T> combine(List<T> list, T... items)

It could be placed in some kind of utility class (and in fact, many libs offer such a method already).

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

PR Review: https://git.openjdk.org/jfx/pull/1296#pullrequestreview-1752157854
PR Review Comment: https://git.openjdk.org/jfx/pull/1296#discussion_r1407290426
PR Review Comment: https://git.openjdk.org/jfx/pull/1296#discussion_r1407307131