Converting to streams

Question

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;
}

score 4 · Answer 1 · answered Jun 07 '18 at 09:34

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.

Nice, I was able to test this on JDK8, although I am still on JDK9 atm so I am not able to verify on JDK10, but I believe you :). +1. — Ousmane D., Jun 07 '18 at 09:39
@Aominè actually this was one of the bullet points we upgraded to jdk-10, this was really important for some pieces of code to be transformed from loops... — Eugene, Jun 07 '18 at 09:41

Ousmane D. · Accepted Answer · 2018-06-07T09:02:31.003

2

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.

edited Jun 07 '18 at 09:02

answered Jun 07 '18 at 08:45

Ousmane D.

54,915
8
91
126

.map(s-> handleAlertAction(reportDTO, consignmentId, s)) this won't be calculated for all the elements in the stream? – Pushparaj Jun 07 '18 at 08:52
@Pushparaj nope, it will stop as soon as it finds the first just like in the code you've shown. – Ousmane D. Jun 07 '18 at 08:53
I think, it is my mis-understanding of streams, correct me, I am thinking that all alerts are flatMapped to getAlertActions and then map and they are filtered based on condition and the first matched is returned. So we are executing the method handleAlertAction(reportDTO, consignmentId, s) every AlertAction – Pushparaj Jun 07 '18 at 08:53

score 1 · Answer 3 · answered Jun 07 '18 at 08:45

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);
}

Ravindra Ranwala · Answer 4 · 2018-06-07T09:03:03.070

1

Try this out,

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

edited Jun 07 '18 at 09:03

answered Jun 07 '18 at 08:54

Ravindra Ranwala

20,744
6
45
63

I suggest you put a linebreak before every dot in a Stream invocation chain. – Christoffer Hammarström Jun 07 '18 at 09:00
I have changed it as per your request. – Ravindra Ranwala Jun 07 '18 at 09:03

Converting to streams

4 Answers4