2017-10-12 2 views
3

J'ai une question qui me dérange depuis un moment sur la méthode de contrôle de flux préférable.Contrôle de flux Java en lançant des exceptions

je rencontre souvent une situation où je dois décider quoi faire en fonction de la valeur de retour d'une méthode qui est null ou not null

J'ai donc deux options que je connais, comment y faire face:

Vérifier null:

public class BasedOnNullFlowControl { 

    public String process(String login) { 
     String redirectUri = getRedirectUri(login); 
     if (redirectUri != null) { 
      return redirectUri; 
     } else { 
      return "Your login is taken"; 
     } 
    } 

    private String getRedirectUri(String login) { 
     Optional<Login> loginFromDb = checkLoginExists(login); 
     if (loginFromDb.isPresent()) { 
      return null; 
     } else { 
      return "some-redirect-url"; 
     } 
    } 

    private Optional<Login> checkLoginExists(String login) { 
     return Optional.empty(); //assume this value comes from some API 
    } 

    private class Login {} 
} 

ou d'interrompre le flux d'exception:

public class ExceptionFlowControl { 

    public String process(String login) { 
     return getRedirectUri(login); 
    } 

    private String getRedirectUri(String login) { 
     Optional<Login> loginFromDb = checkLoginExists(login); 

     loginFromDb.ifPresent(l -> { 
      throw new LoginExistsException(); 
     }); 
     return "some-redirect-url"; 
    } 

    private Optional<Login> checkLoginExists(String login) { 
     return Optional.empty(); 
    } 

    private class LoginExistsException extends RuntimeException { 
    } 

    private class Login {} 
} 

Je sais, que les exceptions ne doivent être utilisées que dans des situations exceptionnelles, mais ma situation n'est pas exceptionnelle et la deuxième solution me semble meilleure, car je peux ajouter un gestionnaire d'exceptions (comme au printemps) Code d'état Http à la fin. La méthode du contrôleur process n'est pas contaminée par des dizaines de vérifications nuls.

Pourriez-vous des gens intelligents s'il vous plaît indiquer quelle solution devrait être utilisée? Ou peut-être qu'il y en a un troisième?

+0

Notez que si vous utilisez 'if (Optional.isPresent())', vous remplacez simplement 'null' par' Optional', mais pas de façon sensée. – Kayaman

+0

Maintenant, regardez le désordre que votre question a causé. – Kayaman

+1

Merci à tous pour votre contribution, maintenant je suis convaincu, qu'il n'y a pas de réponse unique à mon problème et probablement tout dépend de qui fait l'examen du code :) Je voulais juste savoir si le contrôle du flux d'application par les exceptions est toujours un mauvais pratique même si parfois il semble être plus simple et plus propre de cette façon – nibsa

Répondre

1

L'exception permet moins de dépendance entre les parties du code, de sorte que la première façon est assez pauvre. Avec lui, vous devez passer une valeur null chargé avec une signification spéciale (null == login taken) tout le chemin de checkLoginExists() à process.

Cependant, il n'est pas le travail de getRedirectUri() pour lancer l'exception, il devrait être fait dans checkLoginExists(). Je suis douteux d'avoir getRedirectUri() être responsable de vérifier si un login existe ou pas aussi. Sans oublier que l'utilisation de null ou Optional ne vous donne qu'une notion binaire de succès/échec. Que se passe-t-il si le login existe, mais est verrouillé, donc vous devez interdire la connexion ainsi que la création d'un nouvel utilisateur avec le même nom.Vous souhaiterez peut-être rediriger vers une page différente pour indiquer que la connexion est verrouillée. Cependant, ceci est basé sur l'opinion, dépendant fortement des spécificités de la situation et il n'y a pas une règle de coupe claire que vous pourriez suivre. Edit (maintenant que tout ce brouhaha est terminé): Il est généralement admis que le contrôle de flux normal ne doit pas être effectué avec des exceptions. Cependant, la définition de normal n'est pas si facile à définir. Vous ne voudriez pas écrire du code comme

int[] array = ... 
int i = 0; 
try { 
    while(true) { 
     array[i]++; 
     i++; 
    } 
} catch(ArrayIndexOutOfBoundsException e) { 
    // Loop finished 
} 

Toutefois, si vous n'êtes pas en utilisant un exemple ridiculement simple comme ci-dessus, il va dans la zone grise.

Notez également que l'exception n'est pas la même qu'une erreur d'application. Il est impossible (ou du moins pas raisonnable) d'essayer de contourner les exceptions en validant tout. Si nous considérons le cas présenté où un nouvel utilisateur essaie d'enregistrer un nom d'utilisateur, et nous voulons empêcher les noms d'utilisateur en double. Vous pouvez rechercher un doublon et afficher un message d'erreur, mais il est toujours possible que deux demandes simultanées tentent d'enregistrer le même nom d'utilisateur. À ce stade, l'un d'eux obtiendra une exception, car la base de données interdira les doublons (si votre système est sensible de toute façon).

La validation est importante, mais il n'y a rien vraiment exceptionnel à propos des exceptions. Vous devez les manipuler avec élégance, alors pourquoi s'embêter à valider si vous finissez par devoir gérer une exception de toute façon. Si nous prenons un exemple cas de quelque chose que j'ai écrit dans les commentaires, où vous avez besoin pour éviter les doublons et les mots de malédiction que les noms d'utilisateur, vous pouvez venir avec quelque chose comme ça (supposons que cela est dans getRedirectUri():

if(checkForCurseWords(username)) 
    return "login not allowed"; 

try { // We can also omit the try-catch and let the exception bubble upwards 
    createLogin(username); 
} catch(DuplicateException e) { 
    return "login exists"; 
} catch(Exception e) { 
    return "problem with login"; 
} 
return "main page"; 
+0

Je ne suis pas d'accord: les exceptions doivent être utilisées uniquement pour signaler les erreurs d'application qui doivent être corrigées, vous devriez éviter les exceptions en effectuant une validation nécessaire avant d'exécuter la logique. –

+0

Je suis complètement en désaccord avec cette approche inflexible. Dans ce cas, la validation est effectuée, mais des exceptions sont utilisées pour indiquer que la validation échoue. – Kayaman

+0

validation échouer est ... flux de logique normale et ne devrait clairement pas être traitée en lançant une exception, quand vous avez écrit cela, des milliers de petites kites viennent de mourir. –

-1

Cela dépend:

  • Si l'absence de valeur (ou nulle) est une situation normale et attendue dans votre logique, vous ne devez pas jeter exception, mais juste servir en conséquence. Envisagez d'utiliser un modèle null-object - au lieu de retourner null retourner un objet qui représentera un objet "non-value", alors vous pouvez omettre la vérification de null, puisque votre logique devrait gérer cet objet nul en conséquence (souvent vous n'avez pas besoin écrire même une seule ligne de code dans votre logique pour servir ce genre d'objet). Si la situation est que vous ne pouvez vraiment pas procéder avec, vous devriez jeter une exception. Les exceptions doivent être utilisées pour signaler les erreurs dans l'application. Si vous pouvez éviter une exception (par exemple, vérifier si l'utilisateur a spécifié un nom de fichier correct en utilisant file.exists()), vous devriez le faire (à moins qu'il ne soit vraiment hors du flux logique normal et que vous ne puissiez pas continuer).

Dans votre exemple, vous devriez vraiment pas jeter exception, car il est une situation normale (et attendu) qui entrent dans la connexion utilisateur incorrect, ce doit être manipulé par le flux logique normale.

Dans votre situation particulière, pourquoi ne pas simplement retourner l'URL de redirection qui pointe vers une page avec le type de message "L'utilisateur n'existe pas"?

+0

Donc, selon votre suggestion, chaque fois que nous faisons des E/S, nous devons vérifier si le fichier existe (si vous lisez), si nous avons les permissions correctes et toutes les autres situations avant d'effectuer des E/S. – Kayaman

+0

Vous avez également mal interprété le code en question. Il ne s'agit pas de se connecter, mais de créer un nouvel utilisateur et d'empêcher les connexions en double. – Kayaman

+1

Et même si vous vérifiez tout avant de faire le fichier IO, il peut encore échouer (par exemple, le fichier supprimé entre check et IO). – lexicore

1

Première de tous, si vous utilisez optionals vous pouvez éviter les contrôles null tout à fait:

public class BasedOnNullFlowControl { 

    public String process(String login) { 
     String redirectUri = 
     return getRedirectUri(login).orElse("Your login is taken"); 
    } 

    private Optional<String> getRedirectUri(String login) { 
     return checkLoginExists(login).map(ifExists -> "some-redirect-url"); 
    } 

    private Optional<Login> checkLoginExists(String login) { 
     return Optional.empty(); //assume this value comes from some API 
    } 

    private class Login {} 
} 

Ensuite, votre deuxième version ne semble agréable avec des exceptions non vérifiées donc cela se résume à vérification par rapport à des exceptions non vérifiées qui est une sorte de.. question religieuse

Il ya quelques années, j'ai travaillé avec le logiciel où la "génération" précédente de développeurs suivait la religion des exceptions non vérifiées. Comme, "nous ne pouvons généralement pas gérer les exceptions raisonnablement, alors jetons juste des exceptions non vérifiées et avons une routine au niveau supérieur pour les attraper et afficher la boîte de dialogue d'informations d'erreur à l'utilisateur". Cela a échoué spectaculaire et il était extrêmement difficile de le refactoriser. Le code était comme un champ de mines, vous ne pourriez jamais savoir ce qui pourrait ou ne pourrait pas arriver. Après cela, je suis passé complètement à la "religion des exceptions vérifiées". Si vous avez une situation qui serait mieux décrite comme une erreur de programmation (comme, passé null où aucune valeur nulle n'est attendue ou accès à la ressource classpath qui DOIT être là), une erreur d'exécution est appropriée. Dans d'autres cas, modélisez et utilisez les exceptions vérifiées.

+0

Je ne suis pas d'accord: les exceptions doivent être utilisées uniquement pour signaler les erreurs d'application qui doivent être corrigées, vous devriez éviter les exceptions en effectuant une validation nécessaire avant d'exécuter la logique. –

+0

@KrzysztofCichocki Si j'écris une méthode publique qui, par contrat, n'accepte pas les 'null's, je vérifierai typiquement avec 'Objects.requireNonNull'. Si quelqu'un appelle la méthode avec un «null», ils obtiendront un NPE. Dans ma méthode, je ne peux pas valider "avant d'exécuter la logique", c'est un non-sens car la logique est déjà exécutée. Et non, une situation exceptionnelle ne veut pas forcément dire qu'il y a une erreur dans l'application qui devrait être corrigée. Cela signifie qu'il y a une situation que vous ne pouvez pas ou que vous décidez de ne pas gérer. – lexicore

+0

dans une telle situation, cela devrait être décrit dans la méthode doc, donc celui qui invoque votre méthode doit l'utiliser correctement (vérifiez ses valeurs avant d'appeler votre méthode). Ou si la logique de vérification est plus compliquée, vous devriez lui donner une méthode pour vérifier comme ceci est fait dans la classe File par exemple 'file.exists()' –

0

Vous ne recevrez (malheureusement) jamais de réponse claire à ce genre de questions car cela dépend de beaucoup de facteurs et beaucoup de ces facteurs sont basés sur l'opinion.

Je ne vois rien de complètement faux avec vos deux options.En utilisant exception pour le contrôle de flux est considéré comme mauvais practrice en Java (mais il est pas le cas dans une autre langue - comme Python) mais vous a un moment où vous avez dit

Le processus de méthode du contrôleur est pas contaminé par des dizaines null vérifications.

je vous donne une astuce: Lorsque vous hésitez entre deux morceau de code. Demandez-vous quel code quelqu'un qui le regarde pour la première fois comprendra plus facilement.

Dans votre deuxième solution, il est clair que se passe-t-il si la session existe déjà. En six mois, un autre develepper pourrait jeter un oeil à votre code et trouver très facilement ce que vous faites dans un tel cas en cherchant la gestion des exceptions. Compte tenu de l'astuce que je vous ai donné, j'ai trouvé la deuxième option meilleure (à mon humble avis).

Espérons que cela aide.

+0

Je lui donne une réponse claire. Je ne suis pas d'accord avec vous parce que jeter l'exception est la pire des solutions. –

0

Flux de contrôle à l'aide d'exceptions est toujours mauvaise idée, il est ici largement expliqué: https://softwareengineering.stackexchange.com/questions/189222/are-exceptions-as-control-flow-considered-a-serious-antipattern-if-so-why

et ici:

http://wiki.c2.com/?DontUseExceptionsForFlowControl

La ligne beewten utilisant des exceptions vs logique normale devrait être clair, les exceptions ne doivent donc être utilisées que pour de vraies erreurs, et non pour quelque chose qui peut être servi par une logique d'application normale.

@lexicore et @Kayaman, vous pouvez me rétrograder maintenant, je m'en fous. Et ... vérifiez le deuxième lien pour la liste des vrais programmeurs qui disent que vous ne devriez pas utiliser des exceptions pour le contrôle de flux normal. Si votre ego est irrité à cause de cela, je suis vraiment heureux.