On Tue, 28 Nov 2023 00:51:36 GMT, Andy Goryachev <ango...@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

Reply via email to