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

[Ian Graves]

On Thu, 25 Mar 2021 08:40:37 GMT, Tagir F. Valeev <tvaleev at openjdk.org> wrote:

>> Looks good!
>
> 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!

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

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