mbien commented on code in PR #112:
URL:
https://github.com/apache/netbeans-nbpackage/pull/112#discussion_r2310868331
##########
src/main/java/org/apache/netbeans/nbpackage/FileUtils.java:
##########
@@ -263,7 +262,7 @@ public static List<Path> findDirs(Path searchDir, int
searchDepth, String... pat
}
}).allMatch(v -> v);
})) {
- return List.copyOf(stream.collect(Collectors.toList()));
+ return List.copyOf(stream.toList());
Review Comment:
`Collectors.toList` leaves a few things undefined, like mutability, while
`stream.toList()` promises to return a unmodifiable view of something
unspecified (in future potentially value based) which gives the JDK more wiggle
room for optimizations.
> I disagree in classifying Stream::toList as just a language level update,
It often is, because `Stream.toList` is JDK 17 and usually the best practice
now unless more control over the returned list is needed. So code which doesn't
use it likely wasn't upgraded at some point, or it misses a comment why the
edge case of `Collectors` is needed IMO. I saw code once which expects it to be
an `ArrayList` since someone looked at the JDK sources instead of the doc ;)
Are both completely interchangeable? no, but Things usually never are.
I saw also `str.replaceAll("\\\\", ..)` which could be `str.replace("\\",
..)` etc but forgot to add it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@netbeans.apache.org
For additional commands, e-mail: notifications-h...@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
