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