CODEWITHSUNDEEP
javajava-8java-stream
Pushparaj
Pushparaj

Reputation: 1059

Converting to streams

I'd like to convert the following code, which breaks from the outer loop, into Java 8 Streams.

private CPBTuple getTuple(Collection<ConsignmentAlert>  alertsOnCpdDay)
{
    CPBTuple cpbTuple=null;

    OUTER:
    for (ConsignmentAlert consignmentAlert : alertsOnCpdDay) {
        List<AlertAction> alertActions = consignmentAlert.getAlertActions();
        for (AlertAction alertAction : alertActions) {
            cpbTuple = handleAlertAction(reportDTO, consignmentId, alertAction);
            if (cpbTuple.isPresent()) {
                break OUTER;
            }
        }
    }
    return cpbTuple;
}

Upvotes: 3

Views: 132

Answers (4)

Eugene
Eugene

Reputation: 121078

Every answer here uses flatMap, which until java-10 is not lazy. In your case that would mean that alertActions is traversed entirely, while in the for loop example - not. Here is a simplified example:

static class User {
    private final List<String> nickNames;

    public User(List<String> nickNames) {
        this.nickNames = nickNames;
    }

    public List<String> getNickNames() {
        return nickNames;
    }
}

And some usage:

public static void main(String[] args) {
    Arrays.asList(new User(Arrays.asList("one", "uno")))
            .stream()
            .flatMap(x -> x.getNickNames().stream())
            .peek(System.out::println)
            .filter(x -> x.equalsIgnoreCase("one"))
            .findFirst()
            .get();
}

In java-8 this will print both one and uno, since flatMap is not lazy.

On the other hand in java-10 this will print one - and this is what you care about if you want to have your example translated to stream-based 1 to 1.

Upvotes: 4

Ravindra Ranwala
Ravindra Ranwala

Reputation: 21124

Try this out,

alertsOnCpdDay.stream()
    .map(ConsignmentAlert::getAlertActions)
    .flatMap(List::stream)
    .map(alertAction -> handleAlertAction(reportDTO, consignmentId, alertAction))
    .filter(CPBTuple::isPresent)
    .findFirst().orElse(null);

Upvotes: 1

Ousmane D.
Ousmane D.

Reputation: 56489

Something along the lines of this should suffice:

return alertsOnCpdDay.stream()
              .flatMap(s-> s.getAlertActions().stream())
              .map(s-> handleAlertAction(reportDTO, consignmentId, s))
              .filter(s-> s.isPresent())
              .findFirst().orElse(null);

That said, a better option would be to change the method return type to Optional<CPBTuple> and then simply return the result of findFirst(). e.g.

private Optional<CPBTuple> getTuple(Collection<ConsignmentAlert> alertsOnCpdDay) {
    return alertsOnCpdDay.stream()
                  .flatMap(s-> s.getAlertActions().stream())
                  .map(s-> handleAlertAction(reportDTO, consignmentId, s))
                  .filter(s-> s.isPresent())
                  .findFirst();
}

This is better because it better documents the method and helps prevent the issues that arise when dealing with nullity.

Upvotes: 2

Eran
Eran

Reputation: 394146

Since you break out of the loops upon the first match, you can eliminate the loops with a Stream with flatMap, which returns the first available match:

private CPBTuple getTuple(Collection<ConsignmentAlert> alertsOnCpdDay) {
    return alertsOnCpdDay.stream()
                         .flatMap(ca -> ca.getAlertActions().stream())
                         .map(aa -> handleAlertAction(reportDTO, consignmentId, aa))
                         .filter(CPBTuple::isPresent)
                         .findFirst()
                         .orElse(null);
}

Upvotes: 1

Related Questions