Java Lambda for break inside two for loops

2020-03-31 java collections java-8 functional-programming java-stream

I am trying to convert an iterative block of code in Java 8 to functional. The functional approach is unable to find the matching message in the set shared.

List<Optional<Message>> allMessages = new ArrayList<>();

Set<Status> allStatuses = getAllStatuses();

//Iterative : Working
Set<StatusMessage> set = new HashSet<>(STATUS_MESSAGE.values());
for (StatusMessage statusMessage : set) {
    for (Status status : statusMessage.getStatusAndInfo().keySet()) {
        Optional<Message> message = MessageBuilder.createMessage(allStatuses, status, this::createMessage);
        if (message.isPresent()) {
            allMessages.add(message);
            break;
        }
    }
}

//Functional : Not working  - Never adds anything to the 
//map even when matching status is present
STATUS_MESSAGE.values().stream()
        .distinct()
        .map(statusMessage -> statusMessage.getStatusAndInfo().keySet())
        .flatMap(Collection::stream)
        .map(key -> MessageBuilder.createMessage(allStatuses, key, this::createMessage))
        .anyMatch(allMessages::add);

The MessageBuilder.createMessage looks like this:

Optional<Status> matchingStatus = statuses.stream()
                .filter(matchingStatus::equals)
                .findFirst();
System.out.println("Found : " + matchingStatus.toString());
return matchingStatus.flatMap(creator);

Also, for debugging purposes, how can I see what is happening at each step of the stream? The stack in the debugger in intellij wasn't showing anything in the stream.

Answers

This should do it:

STATUS_MESSAGE.values().stream()
        .distinct()
        .forEach(statusMessage ->
            statusMessage.getStatusAndInfo().keySet().stream()
                    .map(status -> MessageBuilder.createMessage(allStatuses, status, this::createMessage))
                    .filter(Optional::isPresent)
                    .findFirst()
                    .ifPresent(allMessages::add)
        );

UPDATE

To build the result list using toList instead of adding to a list:

List<Optional<Message>> allMessages = STATUS_MESSAGE.values().stream()
        .distinct()
        .flatMap(statusMessage ->
            statusMessage.getStatusAndInfo().keySet().stream()
                    .map(status -> MessageBuilder.createMessage(allStatuses, status, this::createMessage))
                    .filter(Optional::isPresent)
                    .limit(1)
        )
        .collect(Collectors.toList());

This should be a comment, but it's too long...


Seems like your MessageBuilder.createMessage method is overcomplicated.

Check below a simplified and more readable version of the same logic:

if (allStatuses.contains(status)) {
    System.out.println("Found : " + status.toString());
    return creator.apply(status);
}
return Optional.empty();

You should not use forEach for accumulating operations, so this should be more idiomatic:

Function<StatusInfo, Optional<Message>> messageForStatus = statusInfo -> 
        statusInfo().keySet().stream()
                    .map(status -> MessageBuilder.createMessage(allStatuses, status, this::createMessage))
                    .filter(Optional::isPresent)
                    .findFirst()
                    .orElse(Optional.empty());

allMessages = STATUS_MESSAGE.values().stream()
        .distinct()
        .map(StatusMessage::getStatusAndInfo)
        .map(messageForStatus)
        .filter(Optional::isPresent)
        .collect(toList());

As a side note, you have too many optionals, you may want to consider unwrapping some earlier, as a list of optionals may just as well be the list of only the present values.

Related