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

Tagir F.Valeev tvaleev at openjdk.java.net
Thu Mar 25 08:43:41 UTC 2021


On Tue, 23 Mar 2021 17:08:53 GMT, Jan Lahoda <jlahoda at openjdk.org> wrote:

>> @lahodaj if you're good with these changes I can go ahead and integrate so you can sponsor.
>
> 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.

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

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


More information about the kulla-dev mailing list