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

Tagir F.Valeev tvaleev at openjdk.java.net
Fri Mar 26 00:26:24 UTC 2021


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 thanks, filed: https://bugs.openjdk.java.net/browse/JDK-8264221. I'm not sure about type and tags.

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

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


More information about the kulla-dev mailing list