1
votes

Java Lambda pour break inside two for boucles

J'essaie de convertir un bloc de code itératif en Java 8 en fonctionnel. L'approche fonctionnelle ne parvient pas à trouver le message correspondant dans l'ensemble partagé.

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

Le MessageBuilder.createMessage ressemble à ceci:

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

java functional-programming java-8 java-stream collections

11 commentaires

anyMatch attend un prédicat et renvoie un booléen. Vous voulez probablement .filter () et ensuite simplement .collect

Qu'entendez-vous par «ne fonctionne pas»? Veuillez détailler

.anyMatch (allMessages :: add); s'arrêtera dès qu'il s'ajoutera avec succès, ce qui se produira à la première itération. Utilisez plutôt findFirst (ou findAny si l'ordre n'est pas pertinent, puis écrivez allMessages = ... stream () ... flatMap () .. filter (Optional :: isPresent). findFir‌ st (). orElse (emptyLis‌ t ());

@VLAZ non, à cause du break , une find * est plus appropriée.

@ njzk2 mais le break est uniquement pour la boucle interne et vous devriez toujours continuer avec l'extérieur ... qui réexécute ensuite l'intérieur. Donc, c'est un élément par boucle externe (au plus).

@VLAZ c'est correct. Actuellement, l'approche fonctionnelle n'est jamais capable de trouver le statut et n'ajoute rien à la liste allMessages

Pourquoi le .distinct () est-il là? STATUS_MESSAGE est-il une énumération? Comment peut-il avoir des valeurs en double?

STATUS_MESSAGE est une carte . La dénomination que j'ai utilisée peut certainement être améliorée.

@tanvi Quelle est la raison du stockage des messages dans List> ? Pourquoi pas List à la place? Est-ce imposé d'une manière ou d'une autre par un framework / une bibliothèque?

@ETO Il y a beaucoup de classes qui renvoient ce type facultatif et le framework prend alors des décisions basées sur ces informations.

@VLAZ bon point, j'ai raté cette partie

3 Réponses :

5
votes

Cela devrait le faire:

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

UPDATE

Pour construire la liste de résultats en utilisant toList code > au lieu d'ajouter à une liste:

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

3 commentaires

forEach ne doit pas être utilisé pour ajouter des éléments à une liste. c'est à cela que sert la collecte (même si dans ce cas cela signifie une carte légèrement plus compliquée au milieu

@ njzk2 Ajout d'une solution pour cela.

belle utilisation de limit , je n'ai pas pensé l'utiliser dans ce cas

2
votes

Cela devrait être un commentaire, mais c'est trop long ...

On dirait que votre méthode MessageBuilder.createMessage est trop compliquée.

Vérifiez ci-dessous un et une version plus lisible de la même logique:

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

1 commentaires

J'avais optimisé et éloigné quelques initiations d'objet de cela et je n'avais jamais réalisé à quel point cela était devenu simple. Merci pour la note @ETO

0
votes

Vous ne devez pas utiliser forEach pour accumuler des opérations, cela devrait donc être plus idiomatique:

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

En remarque, vous avez trop d'options, vous pouvez souhaitez envisager d'en déballer plus tôt, car une liste d'options peut tout aussi bien être la liste des seules valeurs présentes.

0 commentaires