-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: use List.of where possible if only one argument is passed #2468
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am fine with a particular suggested refactoring, but many changes here are unrelated to it
@@ -26,20 +26,13 @@ public static String generate(String url, String prefix, String id) { | |||
|
|||
public static String generate(String url, String prefix, String id, String extension) { | |||
String pathPart = Urls.urlToPathParts(URI.create(url)); | |||
pathPart = pathPart.equals("") ? "(root)" : sanitise(pathPart); | |||
pathPart = pathPart.isEmpty() ? "(root)" : sanitise(pathPart); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally should be isBlank()
so that we handle more edge cases, but out of the scope for the PR title anyway
@@ -158,7 +158,7 @@ public boolean remove(Object o) { | |||
} | |||
|
|||
public boolean containsAll(Collection<?> c) { | |||
return data.containsAll(c); | |||
return new HashSet<>(data).containsAll(c); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HashSet.containsAll tends to be faster.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of course, but you would need to iterate through all the elements at least once to form the new collection. It defeats the purpose of this change for most of the cases
…emock#2468) * chore: use List.of where possible if only one argument is passed * chore: use isBlank
…emock#2468) * chore: use List.of where possible if only one argument is passed * chore: use isBlank
Cleaned up some Arrays.asList that were never adapted/only one arguments, and removed them in case they weren't used at all.
Submitter checklist
#help-contributing
or a project-specific channel like#wiremock-java