Using Streams to replace loops

Question

I have the following code:

boolean found = false;
for (Commit commit : commits.values()) {
    if (commit.getLogMessage().compareTo(commitMessageToSearch) == 0) {
        System.out.println(commit.getSha());
        found = true;
    }
}
if (!found) {
    System.out.println("aksdhlkasj");
}

Is there some way to write this succinctly using streams or anything else in Java

score 0 · Answer 1 · answered Dec 02 '21 at 19:57

0

You can use Stream#filter along with Stream#findFirst.

System.out.println(commits.values().stream()
    .filter(commit -> commit.getLogMessage().compareTo(commitMessageToSearch) == 0)
    .findFirst().map(Commit::getSha).orElse("aksdhlkasj"));

answered Dec 02 '21 at 19:57

Unmitigated

76,500
11
62
80

2

Putting the pipeline in the method parameters impairs readability. Use `ifPresent()` or `ifPresentOrElse()` to call `println()` instead. – erickson Dec 02 '21 at 20:05
1

@Unmitigated is findFirst required here? As I want to print all the match commits and not only the first on – daniel Dec 03 '21 at 06:03
@daniel If you need to print all the matches, then do not call `findFirst`. – Unmitigated Dec 03 '21 at 13:54
Then how `aksdhlkasj` is supposed to be printed if no match is found? – Nowhere Man Dec 03 '21 at 16:22

Nowhere Man · Answer 2 · 2021-12-04T11:17:21.723

The following options are available:

Use StreamEx library (maven repo) and its quasi-intermediate operation StreamEx::ifEmpty:

import one.util.streamex.StreamEx;
// ...
String msgSearch = "commit message";
StreamEx.of(commits.values())
        .filter(c -> c.getLogMessage().compareTo(msgSearch) == 0)
        .ifEmpty("no commit message found: " + msgSearch)
        .map(Commit::getSha)
        .forEach(System.out::println);

Plus: very concise and clean, single run
Minus: use of a 3rd-party lib

Check the contents of the stream using short-circuiting match without redundant collecting into a list just to check if it's empty

if (commits.values().stream()
    .map(Commit::getLogMessage) // equals should be equivalent to compareTo == 0
    .anyMatch(msgSearch::equals)
) {
    commits.values().stream()
        .filter(c -> c.getLogMessage().compareTo(msgSearch) == 0)
        .map(Commit::getSha)
        .forEach(System.out::println);
} else {
    System.out.println("no commit message found: " + msgSearch);
}

Plus: using standard API (no 3-rd party) without side-effect
Minuses: too verbose, in the worst case (long list with the matching element in the end) double iteration of the stream

Use effectively final variable and its setting from the stream as a side effect.

Disclaimer: usage of stateful streams and side effects are not recommended in general:

The best approach is to avoid stateful behavioral parameters to stream operations entirely...
Side-effects in behavioral parameters to stream operations are, in general, discouraged, as they can often lead to unwitting violations of the statelessness requirement, as well as other thread-safety hazards.
A small number of stream operations, such as forEach() and peek(), can operate only via side-effects; these should be used with care

Thus, if AtomicBoolean is carefully selected as a thread-safe and fast container of a boolean value for found flag, which is only set to true from inside the stream (never reset), the following solution may be offered and the risks of safety and performance are mitigated:

// effectively final and thread-safe container boolean
AtomicBoolean found = new AtomicBoolean(); 

commits.values().stream()
    .filter(c -> c.getLogMessage().compareTo(commitMessageToSearch) == 0)
    .map(Commit::getSha)
    .forEach(sha -> {
        found.set(true); 
        System.out.println(sha);
    });

if (!found.get()) {
    System.out.println("none found");
}

Plus: using standard API (no 3rd-party library), single run, no redundant collection
Minus: using side-effect, discouraged by purists, just mimics of for-each loop

I am not a downvoter but I most likely know the reason for it. The Stream violates the "no side effects" principle by modifying the state of `AtomicBoolean`. See my question for more information about this topic as I initially used Streams this way before I learned their real purpose and behavior: https://stackoverflow.com/questions/50977793/is-use-of-atomicinteger-for-indexing-in-stream-a-legit-way — Nikolas Charalambidis, Dec 03 '21 at 20:13
@Nikolas, I wrote a sort of disclaimer about that, and provided a version without the side effect (which is more verbose than the loop solution). OPs question was whether this conversion into streams solution is possible, so I tried to prove the feasibility with the mentioned caveats. The example you referred is using `AtomicInteger`, here the value of `AtomicBoolean` is set to true _once_ and not toggled, so I cannot think of a _real negative_ consequence of this solution which should work for both parallel and sequential streams. — Nowhere Man, Dec 03 '21 at 20:33
@NikolasCharalambidis, I found an interesting solution using `StreamEx::ifEmpty` method though it's a 3-rd party library created to address known limitations of the standard API — Nowhere Man, Dec 04 '21 at 11:19
Interesting, however, there is a still solution using the standard JDK Stream. — Nikolas Charalambidis, Dec 05 '21 at 11:05

Nikolas Charalambidis · Answer 3 · 2021-12-03T22:12:10.100

In case you want to print out all the occurrences and print some String only in case, there was no item found, I am afraid there is no way other than collecting all the relevant sha values into a list and checking for its emptiness using Optional:

commits.values()
       .stream()
       .filter(commit -> commit.getLogMessage().compareTo(commitMessageToSearch) == 0)
       .map(Commit::getSha)
       .peek(System.out::println)
       .collect(Collectors.collectingAndThen(Collectors.toList(), Optional::of))
       .filter(List::isEmpty)
       .ifPresent(emptyList -> System.out.println("aksdhlkasj"));

Although the intermediate output Optional<List<?>> is against common sense, it helps the further processing using Optinal and comfortably handling the case the list is empty.

However, this form is in my opinion more readable:

List<String> shaList = commits.values()
        .stream()
        .filter(commit -> commit.getLogMessage().compareTo(commitMessageToSearch) == 0)
        .map(Commit::getSha)
        .peek(System.out::println)
        .collect(Collectors.toList());
        
if (shaList.isEmpty()) {
    System.out.println("aksdhlkasj");
}

This is actually similar to existing answer by @Unmitigated and it does not print _all_ matching SHAs as in the loop solution. Maybe someday a method to handle/log an empty stream will finally be added to Stream API to get rid of this clumsiness (e.g., like `forEachOrEmpty` accepting two consumers). — Nowhere Man, Dec 03 '21 at 20:59
@AlexRudenko It seems like @Unmitigated had the same understanding of the question but my answer was different anyway. I initially thought the OP missed a `break;`. Now I understand the question and edited my answer. Please, take a look if you are interested. — Nikolas Charalambidis, Dec 03 '21 at 22:16

Using Streams to replace loops

3 Answers3