b0c1 commented on code in PR #2744:
URL: https://github.com/apache/fineract/pull/2744#discussion_r1022109497
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/jobs/filter/LoanCOBApiFilter.java:
##########
@@ -133,17 +133,9 @@ private void reject(Long loanId, HttpServletResponse
response, int status) throw
private Long getLoanId(boolean isGlim, Supplier<Stream<String>>
streamSupplier) {
if (!isGlim) {
- if (streamSupplier.get().count() >= LOAN_ID_INDEX_IN_URL + 1) {
- return
Long.valueOf(streamSupplier.get().skip(LOAN_ID_INDEX_IN_URL).findFirst().get());
- } else {
- return null;
- }
+ return
streamSupplier.get().skip(LOAN_ID_INDEX_IN_URL).findFirst().map(Long::valueOf).orElse(null);
Review Comment:
I checked the original test suite, and I think we can't write the test to
the else branch because the `isOnApiList` will ignore the request if it's not
match the pattern `/loans/\d+.*` or `/loans/glimAccount/\d+.*`.
The modification does two things:
- remove the unnecessary count check, because if the skip is greater than
the stream, it will return an empty stream.
- Sonar marked the code with major issue because the `findFirst` will
return with Optional, but not checked the result before you access with `get`.
So it's not modify the current behavior, just reduce the code and
eliminating the Sonar issues.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
