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

Glavo duke at openjdk.org
Wed Feb 1 15:15:03 UTC 2023


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:
> 
> ```java
>         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
> 
> ```java
>         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 `List` **is** unmodifiable."

I have considered this, but I didn't make this change because I was worried that there would be less descriptive information when null was encountered.

If this is not a problem, I am very happy to be able to carry out such refactoring.

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

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


More information about the openjfx-dev mailing list