RFR: 8301604: Replace Collections.unmodifiableList with List.of

[Nir Lisker]

On Thu, 26 Jan 2023 05:30:56 GMT, Glavo <duke at openjdk.org> wrote:

> `List.of` is cleaner, and can slightly reduce the memory footprint for lists of one or two elements.
> 
> Because `List.of` can only store non-null elements, I have only replaced a few usage.
> 
> Can someone open an Issue on JBS for me?

### FileChooser

`List.of` and `List.copyOf` already check for a `null` argument and `null` elements. This means that `validateArgs` only needs to check the `length` of `extensions` and for an empty element. Since the only difference between the constructors is taking an array vs. taking a list, once a list is created from the array, the array constructor can call the list constructor.

I suggest the following refactoring:

        public ExtensionFilter(String description, String... extensions) {
            this(description, List.of(extensions));
        }

        public ExtensionFilter(String description, List<String> extensions) {
            var extensionsList = List.copyOf(extensions);
            validateArgs(description, extensionsList);
            this.description = description;
            this.extensions = extensionsList;
        }

Note that `List.copyOf` will not create another copy if it was called from the array constructor that already created an unmodifiable `List.of`.

Now validation can be reduced to

        private static void validateArgs(String description, List<String> extensions) {
            Objects.requireNonNull(description, "Description must not be null");

            if (description.isEmpty()) {
                throw new IllegalArgumentException("Description must not be empty");
            }

            if (extensions.isEmpty()) {
                throw new IllegalArgumentException("At least one extension must be defined");
            }

            for (String extension : extensions) {
                if (extension.isEmpty()) {
                    throw new IllegalArgumentException("Extension must not be empty");
                }
            }
        }


Aditionally, please add empty lines after the class declarations of `FileChooser` and `ExtensionFilter`.

Also please correct the typo in `getTracks`: "The returned <code>List</code> **is** unmodifiable."

modules/javafx.graphics/src/main/java/com/sun/javafx/iio/common/ImageDescriptor.java line 41:

> 39:         this.extensions = List.of(extensions);
> 40:         this.signatures = List.of(signatures);
> 41:         this.mimeSubtypes = List.of(mimeSubtypes);

While this is not equivalent since changing the backing array will not change the resulting list anymore, I would consider the old code a bug and this the correct fix.

Note that in `FileChooser` care is taken to create a copy of the supplied array before using it to create a list.

Additionally, care must be taken that all the callers don't have `null` elements in the arrays they pass. I checked it and it's fine (and should probably be disallowed, which is good now).

By the way, this class should be a `record`, and all its subtypes, which are not really subtypes but just configured instances, should be modified accordingly. This is out of scope though.

modules/javafx.media/src/main/java/com/sun/media/jfxmedia/Media.java line 100:

> 98:                 returnValue = null;
> 99:             } else {
> 100:                 returnValue = List.copyOf(tracks);

This is fine because `addTrack` checks for `null` elements.

modules/javafx.media/src/main/java/com/sun/media/jfxmedia/Media.java line 103:

> 101:             }
> 102:         }
> 103:         return returnValue;

This method can be reduced to

    public List<Track> getTracks() {
        synchronized (tracks) {
            return tracks.isEmpty() ? null : List.copyOf(tracks);
        }
    }

though I find it highly questionable that it returns `null` for an empty list instead of just an empty list. There are 2 use cases of this method and both would do better with just an empty list.

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

Changes requested by nlisker (Reviewer).

PR: https://git.openjdk.org/jfx/pull/1012