RFR: JDK-8263411: Convert jshell tool to use Stream.toList() [v3]

[Tagir F.Valeev]

On Thu, 25 Mar 2021 14:44:19 GMT, Ian Graves <igraves at openjdk.org> wrote:

>> Sorry to be late but the change in SnippetMaps.java looks really confusing to me:
>> 
>>         return Stream.concat(Stream.of("java.lang"), pkgs)
>>                      .filter(ipkg -> !ipkg.isEmpty() && ipkg.equals(pkg))
>>                      .map(ipkg -> full.substring(pkg.length() + 1))
>>                      .findFirst()
>>                      .orElse(full);
>> The elements in the stream after `map` do not depend on the previous stream content. To me, it looks a misuse of stream API. Here, `anyMatch` scenario would look better:
>>         return Stream.concat(Stream.of("java.lang"), pkgs)
>>                      .anyMatch(ipkg -> !ipkg.isEmpty() && ipkg.equals(pkg)) 
>>                      ? full.substring(pkg.length() + 1) : full;
>> 
>> Also, it looks like the emptiness check could be applied to `pkg` variable, rather than to every stream element (as we require equality of `ipkg` and `pkg` after that:
>> 
>>         return !pkg.isEmpty() && Stream.concat(Stream.of("java.lang"), pkgs).anyMatch(pkg::equals) 
>>                    ? full.substring(pkg.length() + 1) : full;
>> 
>> Sorry if I'm missing something.
>
> @amaembo Since the issue is now closed and these suggestions don't have to do with correctness, I'd suggest creating a new issue and PR to put these proposed improvements forward. Thanks!

@igraves could you please take a look? https://github.com/openjdk/jdk/pull/3209 Thanks!

-------------

PR: https://git.openjdk.java.net/jdk/pull/2979