Comment utiliser Java 8 Optionals en effectuant une action si les trois sont présents?

Question

J'ai quelques (simplifié) du code qui utilise Java Options:

Optional<User> maybeTarget = userRepository.findById(id1);
Optional<String> maybeSourceName = userRepository.findById(id2).map(User::getName);
Optional<String> maybeEventName = eventRepository.findById(id3).map(Event::getName);

maybeTarget.ifPresent(target -> {
    maybeSourceName.ifPresent(sourceName -> {
        maybeEventName.ifPresent(eventName -> {
            sendInvite(target.getEmail(), String.format("Hi %s, $s has invited you to $s", target.getName(), sourceName, meetingName));
        }
    }
}

Inutile de dire que cela ressemble et se sent mal. Mais je ne peux pas penser à une autre façon de faire de moins en moins imbriquées et plus lisible. J'ai envisagé de le streaming les 3 Options, mais rejeté l'idée que le fait de faire un .filter(Optional::isPresent) puis un .map(Optional::get) se sent encore pire.

Donc, il y a un mieux, plus "Java 8" ou "Facultatif-alphabètes" façon de faire face à cette situation (essentiellement de multiples Options nécessaires pour calculer une opération finale)?

Answer 1

Je pense que diffuser les trois Optional s est une overkill, pourquoi pas le simple

 if (maybeTarget.isPresent() && maybeSourceName.isPresent() && maybeEventName.isPresent()) {
  ...
}
 

À mes yeux, cela indique plus clairement la logique conditionnelle par rapport à l'utilisation de l'API de flux.

Answer 2

En utilisant une fonction d'assistance, les choses deviennent au moins un peu imbriquées:

 @FunctionalInterface
interface TriConsumer<T, U, S> {
    void accept(T t, U u, S s);
}

public static <T, U, S> void allOf(Optional<T> o1, Optional<U> o2, Optional<S> o3,
       TriConsumer<T, U, S> consumer) {
    o1.ifPresent(t -> o2.ifPresent(u -> o3.ifPresent(s -> consumer.accept(t, u, s))));
}
 

 allOf(maybeTarget, maybeSourceName, maybeEventName,
    (target, sourceName, eventName) -> {
        /// ...
});
 

L'inconvénient évident est que vous aurez besoin d'une surcharge de la fonction d'assistance distincte pour chaque nombre différent de Optional s.

Answer 3

Depuis le premier code est exécuté pour ses effets secondaires (envoi d'un email), et non pas de faire l'extraction ou la génération d'une valeur, le sous - ifPresent des appels semblent appropriées. Le code d'origine ne semble pas trop mal, et il semble en effet plutôt mieux que certaines des réponses qui ont été proposées. Cependant, la déclaration des lambdas et les variables locales de type Optional ne semblent ajouter une bonne quantité de désordre.

Tout d'abord, je vais prendre la liberté de modifier le code d'origine en l'enveloppant dans une méthode, en donnant les paramètres de nice noms, et de faire jusqu'à un certain type de noms. Je n'ai aucune idée si le code est comme cela, mais cela ne devrait pas vraiment être surprenant pour quelqu'un.

// original version, slightly modified
void inviteById(UserId targetId, UserId sourceId, EventId eventId) {
    Optional<User> maybeTarget = userRepository.findById(targetId);
    Optional<String> maybeSourceName = userRepository.findById(sourceId).map(User::getName);
    Optional<String> maybeEventName = eventRepository.findById(eventId).map(Event::getName);

    maybeTarget.ifPresent(target -> {
        maybeSourceName.ifPresent(sourceName -> {
            maybeEventName.ifPresent(eventName -> {
                sendInvite(target.getEmail(), String.format("Hi %s, %s has invited you to %s",
                                                  target.getName(), sourceName, eventName));
            });
        });
    });
}

J'ai joué un peu avec les différents refactorings, et j'ai trouvé que l'extraction de l'intérieure de la déclaration lambda dans sa propre méthode qui fait le plus de sens pour moi. Compte tenu de la source et de la cible les utilisateurs et à un événement (non Facultatif des trucs, il envoie un courrier à ce sujet. C'est le calcul qui doit être effectué après tout, l'option n'a été traité. J'ai aussi déplacé l'extraction des données (email, nom) ici au lieu de le mélanger avec l'Option de traitement dans la couche externe. Encore une fois, cela fait du sens pour moi: envoyer du courrier à partir de la source à la cible à propos de l'événement.

void setupInvite(User target, User source, Event event) {
    sendInvite(target.getEmail(), String.format("Hi %s, %s has invited you to %s",
               target.getName(), source.getName(), event.getName()));
}

Maintenant, nous allons traiter avec l'option de trucs. Comme je l'ai dit ci-dessus, ifPresent est le chemin à parcourir, car nous voulons faire quelque chose avec des effets secondaires. Il fournit également un moyen d'extraire la valeur d'une Option et le lier à un nom, mais seulement dans le contexte d'une expression lambda. Depuis que nous voulons faire cela pour trois différentes Options, d'imbrication est appelé pour. L'imbrication permet aux noms de l'extérieur lambdas à être capturé par intérieure lambdas. Cela nous permet de lier les noms de valeurs extraites de la Options -- mais seulement si elles sont présentes. Cela ne peut pas vraiment être fait avec une chaîne linéaire, puisque certains intermédiaires structure de données comme un n-uplet serait nécessaire pour construire les résultats partiels.

Enfin, au plus profond de la lambda, nous appelons la méthode d'assistance définies ci-dessus.

void inviteById(UserId targetId, UserId sourceID, EventId eventId) {
    userRepository.findById(targetId).ifPresent(
        target -> userRepository.findById(sourceID).ifPresent(
            source -> eventRepository.findById(eventId).ifPresent(
                event -> setupInvite(target, source, event))));
}

Notez que j'ai incorporé les objectifs optionnels au lieu de les garder dans des variables locales. Cela révèle l'imbrication de la structure un peu mieux. Il fournit également de "court-circuit" de l'opération si l'une des recherches n'a pas trouvé quoi que ce soit, depuis ifPresent n'a tout simplement rien sur un vide en Option.

Il est encore un peu dense à mes yeux, bien que. Je pense que la raison en est que ce code dépend encore de certains des référentiels externes sur lesquels faire les recherches. C'est un peu mal à l'aise d'avoir cette mélangés avec l'Option de traitement. Une possibilité est tout simplement pour extraire les recherches dans leurs propres méthodes d' findUser et findEvent. Ces sont assez évident, donc je ne vais pas les écrire. Mais, si cela était fait, le résultat serait:

void inviteById(UserId targetId, UserId sourceID, EventId eventId) {
    findUser(targetId).ifPresent(
        target -> findUser(sourceID).ifPresent(
            source -> findEvent(eventId).ifPresent(
                event -> setupInvite(target, source, event))));
}

Fondamentalement, ce n'est pas différent de l'original du code. C'est subjectif, mais je pense que je préfère cela à l'origine du code. Il a la même, assez simple structure, même si imbriquée au lieu de la typique linéaire de la chaîne de traitement en Option. Ce qui est différent, c'est que les recherches sont faites conditionnellement à l'intérieur de traitement en Option, au lieu d'être fait avant, stockées dans des variables locales, et ensuite de faire uniquement sous réserve de l'extraction des valeurs Optionnelles. Également, je l'ai séparé de manipulation de données (extraction de l'e-mail et le nom, l'envoi de message) dans une méthode distincte. Cela permet d'éviter de mélanger de manipulation de données avec l'Option de traitement, qui, je pense, a tendance à confondre les choses, si nous avons affaire à de multiples Facultatif instances.

Answer 4

Que diriez-vous quelque chose comme ça

  if(Stream.of(maybeTarget, maybeSourceName,  
                        maybeEventName).allMatch(Optional::isPresent))
  {
   sendinvite(....)// do get on all optionals.
  }
 

Ayant dit cela. Si votre logique à rechercher dans la base de données consiste uniquement à envoyer du courrier, alors si maybeTarget.ifPresent() est faux, il ne sert à rien d'extraire les deux autres valeurs, n'est-ce pas?. Je crains que cette sorte de logique ne puisse être réalisée que par le biais de déclarations traditionnelles, sinon.

Answer 5

Je pense que vous devriez envisager de prendre une autre approche.

J'aimerais commencer par ne pas émettre les trois appels à la DB au début. Au lieu de cela, je voudrais émettre le 1er requête et seulement si le résultat est présent, je l'avais question de la 2e. Je ne puis appliquer le même raisonnement à l'égard de la 3ème requête et enfin, si le dernier résultat est également présent, je souhaitez envoyer l'invitation. Cela permettrait d'éviter les appels inutiles à la DB, soit lorsque l'un des deux premiers résultats n'est pas présent.

Afin de rendre le code plus lisible, testable et maintenable, je voudrais aussi extraire chaque DB appel à sa propre méthode, enchaînant avec Optional.ifPresent:

public void sendInvite(Long targetId, Long sourceId, Long meetingId) {
    userRepository.findById(targetId)
        .ifPresent(target -> sendInvite(target, sourceId, meetingId));
}

private void sendInvite(User target, Long sourceId, Long meetingId) {
    userRepository.findById(sourceId)
        .map(User::getName)
        .ifPresent(sourceName -> sendInvite(target, sourceName, meetingId));
}

private void sendInvite(User target, String sourceName, Long meetingId) {
    eventRepository.findById(meetingId)
        .map(Event::getName)
        .ifPresent(meetingName -> sendInvite(target, sourceName, meetingName));
}

private void sendInvite(User target, String sourceName, String meetingName) {
    String contents = String.format(
        "Hi %s, $s has invited you to $s", 
        target.getName(), 
        sourceName, 
        meetingName);
    sendInvite(target.getEmail(), contents);
}