On Thu, 26 Jan 2023 05:30:56 GMT, Glavo <d...@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
