Cleanup Only - Replace calls to Arrays.asList with too few arguments.

[currantw]

Description

Cleanup Only - Replace calls to Arrays.asList with too few arguments.

From IntelliJ IDEA:

Related Issues

None

Check List

• N/A - New functionality includes testing.
• N/A - New functionality has been documented.
• N/A - New functionality has javadoc added.
• N/A - New functionality has a user manual doc added.
• N/A - API changes companion pull request created.
• Commits are signed per the DCO using --signoff.
• N/A - Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[YANG-DB]

can u use static import for the Collections.singletonList & List.of methods ?

[LantaoJin]

This is just a one-time thing, we can't prevent the Collections API from being used again through this PR. If only for this reason, I don't recommend converting Collections.xx to List.of. It's okey to replace calls for Arrays.asList.

Removing dependencies will make the code more readable and easier to use - and I would encourage others to avoid adding unnecessary dependencies to files. I find that the List.of method is more modern and versatile than the Collections apis - but they are only subtle differences between the two.

For the sake of reducing feature changes, we can revert the Collections.xx to List.of, but I think this is a positive change.

For single responsibility, let's focus on replacing calls to Arrays in this PR.

Create a new issue for replacing Collections.xx including follows items to discuss:

1. Providing similar code changes in other open source projects.
2. Any documents or articles related to replacing Collections.xx.
3. As long-term, a checkstyle for Java to emit warning on the usage of Collections.xx.

[LantaoJin]

Let's focus on cleanup calls to Arrays.asList in this PR.

[currantw]

Let's focus on cleanup calls to Arrays.asList in this PR.

Thanks @LantaoJin. I've gone through and reverted all of the changes that aren't specifically related to Arrays.asList. I don't think I have enough experience in Java to do a full comparison of Collections.xx and alternatives, nor do I really feel strongly enough about it to pursue it.

Please let me know if you have any further review comments.

[YANG-DB]

@currantw LGTM

[LantaoJin]

I only reviewed the product code. Looks good. But there are still some Arrays.asList calls in test code. Please search all code base and check them. Make sure only immutable list should be replaced.

[LantaoJin]

These two are missing.
Please search all code base again.

[LantaoJin]

ditto in L66

[currantw]

I only reviewed the product code. Looks good. But there are still some Arrays.asList calls in test code. Please search all code base and check them. Make sure only immutable list should be replaced.

Thanks @LantaoJin. As you'll see from the description, the original intent of this cleanup PR was only to replace Arrays.asList with List.of for cases where the list has zero or one elements only, which trigger the corresponding warning (if enabled) in IntelliJ IDEA.

My (limited) understanding is that, for lists with two or fewer elements, List.of uses a field-based (rather than array-based) implementation, which is (slightly) more space-efficient and quicker. (For larger lists, both option are array-based, so there is no functional difference). There are other potential advantages to List.of like immutability, thread-safety, and not prohibiting null elements, though I'm not sure if any of these are relevant.

That being said, I am happy to go ahead with switching additional calls from Arrays.asList to List.of, if you think this is desirable. I just want to make sure: do you want me to replace calls to Arrays.asList only in files where there have already been changes as part of this PR (which would remove the dependence on Arrays in these classes), or to do this more generally across the code base (except in cases where the list needs to be mutable or support null values, of course).

Please let me know how you want to proceed. Thanks!

[LantaoJin]

Actually I don't think we have any necessary reason to refactor the calls in Tests. They are tests, they are not called in product. The main difference between Arrays.asList() and Collections.xx or List.of are the previous API return a mutable list, but the rests return immutable list. Existing Arrays.asList in tests won't have memory problem. And this PR is not a long-term solution, you can't prevent new usage added in future. So If you want to complete this contribution. My suggestion is just replace the product code only with the IntelliJ IDEA's suggestion:

List<String> empty = Arrays.asList();
List<String> one = Arrays.asList("one");
Applied to
List<String> empty = Collections.emptyList();
List<String> one = Collections.singletonList("one");

[YANG-DB]

Actually I don't think we have any necessary reason to refactor the calls in Tests. They are tests, they are not called in product. The main difference between Arrays.asList() and Collections.xx or List.of are the previous API return a mutable list, but the rests return immutable list. Existing Arrays.asList in tests won't have memory problem. And this PR is not a long-term solution, you can't prevent new usage added in future. So If you want to complete this contribution. My suggestion is just replace the product code only with the IntelliJ IDEA's suggestion:

List<String> empty = Arrays.asList();
List<String> one = Arrays.asList("one");
Applied to
List<String> empty = Collections.emptyList();
List<String> one = Collections.singletonList("one");

@LantaoJin thanks for your feedback !
@currantw I think @LantaoJin has a good point here - we need to prioritize changes according to their expected outcome benefits - I appreciate you’ve taken time to address the IDE's suggestions

For better clarity and planning onward - before starting the actual coding work - lets first create a detailed issue with the following details:

• issue type (Bug / feature / enhancement )
• in this case the enhancement - explain the need for the change and how it effects
• detail the expected work - in case of large changed please split the issue into smaller tasks

Thanks again and for everyone's work and contributions - we are making great progress !!

[currantw]

Actually I don't think we have any necessary reason to refactor the calls in Tests. They are tests, they are not called in product. The main difference between Arrays.asList() and Collections.xx or List.of are the previous API return a mutable list, but the rests return immutable list. Existing Arrays.asList in tests won't have memory problem. And this PR is not a long-term solution, you can't prevent new usage added in future. So If you want to complete this contribution. My suggestion is just replace the product code only with the IntelliJ IDEA's suggestion:

Thanks @LantaoJin and @YANG-DB. Based on the feedback, I'm just going to close this PR. Think I've dedicated enough time to this already! Will make sure to raise a dedicated issue in the future.