RFR: 1256: Use stream api to simplify the csr issue lookup [v2]
Erik Joelsson
erikj at openjdk.java.net
Tue Nov 30 23:24:52 UTC 2021
On Tue, 30 Nov 2021 16:07:10 GMT, Guoxiong Li <gli at openjdk.org> wrote:
>> Hi all,
>>
>> This patch adds the common methods `Issue#linksWithRelationships`, `Issue#csrLink` and `Issue#csrIssue` and refactors the code to reduce the loop nested statements by using these common methods.
>>
>> All the existing tests passed.
>>
>> Best Regards,
>> -- Guoxiong
>
> Guoxiong Li has updated the pull request incrementally with one additional commit since the last revision:
>
> Use stream api instead of the encapsulated method.
I think this kind of refactoring is good enough for now. I still think the Issue interface should get refactored along the lines I described at some point, so that concepts like different types of links can be handled by a single appropriate class/interface. The clash with the other "Issue" class which represents something completely different is very unfortunate as well.
bots/pr/src/main/java/org/openjdk/skara/bots/pr/CSRCommand.java line 165:
> 163: }
> 164:
> 165: var csr = jbsIssue.get().links().stream()
This particular pattern now occurs twice in this file. Perhaps worth moving to a private method at least, to reduce some duplication.
bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckRun.java line 142:
> 140: .filter(link -> link.relationship().isPresent() && "csr for".equals(link.relationship().get()))
> 141: .findAny().flatMap(Link::issue).orElse(null);
> 142: if (csr == null) {
I just noticed this pattern being used a lot. Instead of .orElse(null), I would prefer if you let `csr` be an optional and used `csr.isPresent()` in the if statement. You will then need to use `csr.get()` to actually get the value, but this is safe as it's protected by the condition on isPresent(). If the variable in question is used a lot, you can choose resolve it with .get() into a new variable at the top of the if/else block.
This applies to several places where the same pattern is used.
-------------
PR: https://git.openjdk.java.net/skara/pull/1248
More information about the skara-dev
mailing list