Devrais-je revenir tôt d'une fonction ou utiliser une instruction if? [fermé]

303

J'ai souvent écrit ce type de fonction dans les deux formats, et je me demandais si un format était préféré à un autre et pourquoi.

public void SomeFunction(bool someCondition)
{
    if (someCondition)
    {
        // Do Something
    }
}

ou

public void SomeFunction(bool someCondition)
{
    if (!someCondition)
        return;

    // Do Something
}

Je code habituellement avec le premier puisque c'est ainsi que mon cerveau fonctionne lors du codage, même si je pense que je préfère le second, car il gère immédiatement toute erreur et me permet de lire plus facilement.

Rachel
la source
9
Je suis un peu en retard pour cette discussion donc je ne vais pas le mettre dans une réponse; J'y ai aussi pensé il y a deux ans: lecterror.com/articles/view/code-formatting-and-readability Je trouve la seconde plus facile à lire, à modifier, à maintenir et à déboguer. Mais peut-être que c'est juste moi :)
dr Hannibal Lecter
4
maintenant, cette question est un bon exemple d'une question d'opinion
Rudolf Olah,
2
alors que se passe-t-il s'il n'y a pas de preuve absolue dans l'une ou l'autre direction? Lorsque suffisamment d'arguments sont fournis dans un sens et dans l'autre et qu'il y a un vote si la réponse est correcte, cela le rend très utile. Je trouve les dernières questions de ce type préjudiciables à la valeur de ce site.
GSF
9
Et j'aime les questions et réponses basées sur les opinions. Ils me disent ce que la majorité préfère et cela me permet d'écrire le code pour que les autres puissent le lire.
Zygimantas

Réponses:

401

Je préfère le deuxième style. Commencez par supprimer les cas non valides, soit en supprimant les exceptions, soit en levant des exceptions, insérez une ligne vide, puis ajoutez le corps "réel" de la méthode. Je trouve cela plus facile à lire.

Maçon Wheeler
la source
7
Smalltalk appelle ces "clauses de garde". Du moins, c'est ce que Kent Beck appelle ces méthodes dans les modèles de meilleures pratiques Smalltalk; Je ne sais pas si c'est le langage courant.
Frank Shearar
154
Quitter tôt vous permet de sortir des objets de votre stack mental limité. :)
Joren
50
J'ai déjà entendu parler de "modèle de videur" - éliminez les cas graves avant qu'ils n'entrent dans la porte.
RevBingo
14
J'irais même jusqu'à dire que c'est définitivement la SEULE chose à faire.
Oliver Weiler
38
De plus, vous ne continuez pas à ajouter des retraits si vous supprimez les cas de frontière au début.
Doppelgreener
170

Certainement le dernier. Le premier ne semble pas mauvais en ce moment, mais quand vous obtenez un code plus complexe, je ne peux pas imaginer que quiconque puisse penser ceci:

public int SomeFunction(bool cond1, string name, int value, AuthInfo perms)
{
    int retval = SUCCESS;
    if (someCondition)
    {
        if (name != null && name != "")
        {
            if (value != 0)
            {
                if (perms.allow(name)
                {
                    // Do Something
                }
                else
                {
                    reval = PERM_DENY;
                }
            }
            else
            {
                retval = BAD_VALUE;
            }
        }
        else
        {
            retval = BAD_NAME;
        }
    }
    else
    {
        retval = BAD_COND;
    }
    return retval;
}

est plus lisible que

public int SomeFunction(bool cond1, string name, int value, AuthInfo perms)
{
    if (!someCondition)
        return BAD_COND;

    if (name == null || name == "")
        return BAD_NAME;

    if (value == 0)
        return BAD_VALUE;

    if (!perms.allow(name))
        return PERM_DENY;

    // Do something
    return SUCCESS;
}

Je reconnais pleinement que je n’ai jamais compris l’avantage des points de sortie uniques.

Jason Viers
la source
20
L'avantage d'un seul point de sortie est qu'il n'y a qu'un seul point de sortie! Avec votre exemple, il y a plusieurs points qui pourraient revenir. Avec une fonction plus complexe, cela pourrait se transformer en un point de départ recherché lorsque le format de la valeur de retour change. Bien sûr, il y a des moments où forcer un seul point de sortie n'a pas de sens.
JohnL
71
@JohnL Les grandes fonctions sont le problème, pas de multiples points de sortie. Sauf si vous travaillez dans un contexte où un appel de fonction supplémentaire va ralentir votre code énormément, bien sûr ...
Dan Rosenstark
4
@Yar: C'est vrai, mais le point reste. Je ne vais pas essayer de convertir qui que ce soit, et je soulignais simplement l'avantage de choisir le moins de points de sortie possible (et l'exemple de Jason était un peu un homme de paille). Juste la prochaine fois que vous vous démêlerez en essayant de comprendre pourquoi SomeFunction renvoie parfois des valeurs impaires, vous voudrez peut-être pouvoir ajouter un appel d'enregistrement juste avant le retour. Beaucoup plus facile à déboguer s'il n'y a qu'un seul bugger! :)
JohnL
76
@Jason Viers Comme si un seul return value;aide!?! Ensuite, il faut en chasser une demi-douzaine value = ..., avec le désavantage de ne jamais être sûr que la valeur ne sera pas modifiée entre cette cession et la déclaration finale. Au moins un retour immédiat est clair que rien ne changera plus le résultat.
Sjoerd
5
@ Sjoed: J'appuie Sjoerd ici. Si vous souhaitez enregistrer le résultat, vous pouvez vous connecter sur le site de l'appelant. Si vous voulez connaître la raison, vous devez vous connecter à chaque point de sortie / affectation pour que les deux soient identiques dans cet aspect.
Matthieu M.
32

Cela dépend - En général, je ne vais pas faire tout ce qui est en mon pouvoir pour essayer de déplacer un tas de code afin de sortir de la fonction plus tôt que prévu - le compilateur s'en chargera généralement pour moi. Cela dit, s’il existe certains paramètres de base dont j’ai besoin et que je ne peux pas continuer sinon, j’interviendrai tôt. De même, si une condition génère un ifbloc géant en fonction, je l'aurai aussi tôt.

Cela dit, cependant, si une fonction nécessite des données lors de son appel, je vais généralement lancer une exception (voir exemple) au lieu de simplement y retourner.

public int myFunction(string parameterOne, string parameterTwo) {
  // Can't work without a value
  if (string.IsNullOrEmpty(parameterOne)) {
    throw new ArgumentNullException("parameterOne");
  } 
  if (string.IsNullOrEmpty(parameterTwo)) {
    throw new ArgumentNullException("parameterTwo");
  }

  // ...      
  // Do some work
  // ...

  return value;
}
rjzii
la source
9
Et si le résultat final est maintenable, qui se soucie du style choisi?
Jeff Siver
3
@ Jeff Siver - C’est pourquoi il s’agit là d’une question de style "guerre sainte". C’est finalement une question de préférence personnelle et de ce que dit un guide de style interne.
Rjzii
1
Ce qu'il faut retenir ici, c'est qu'il lance une exception au lieu de revenir plus tôt. La valeur de retour ne doit pas être réutilisée pour les contrôles de validité. Et si vous aviez différentes conditions et que vous vouliez que le code utilisant la méthode sache pourquoi il a échoué? Soudain, vous pouvez renvoyer des données commerciales réelles, rien (résultat vide) ou de nombreuses chaînes, codes, chiffres, ... différents à partir d'une seule méthode. Juste pour décrire pourquoi cela a échoué. Non merci.
DanMan
Qu'en est-il de la complexité cyclomatique comme le suggère mon compilateur? N'est-il pas préférable de ne pas imbriquer le code si on peut l'aider?
l -'''''''--------- '' '' '' '' '' ''
24

Je préfère le retour rapide.

Si vous avez un point d'entrée et un point de sortie, vous devez toujours suivre l'intégralité du code dans votre tête jusqu'au point de sortie (vous ne savez jamais si un autre morceau de code ci-dessous fait quelque chose d'autre, avoir à suivre jusqu’à exister). Vous ne le faites pas quelle branche détermine le résultat final. C'est difficile à suivre.

Avec une entrée et plusieurs existantes, vous revenez lorsque vous avez votre résultat et vous ne perdez pas la peine à la suivre pour vous assurer que personne ne fait rien d'autre (car il n'y aura rien d'autre depuis votre retour). C'est comme si le corps de la méthode était scindé en plusieurs étapes, chacune d'elles avec la possibilité de renvoyer le résultat ou de laisser la prochaine étape tenter sa chance.

utilisateur7197
la source
13

En programmation C, où vous devez nettoyer manuellement, il y a beaucoup à dire pour un retour en un point. Même s'il n'est pas nécessaire pour le moment de nettoyer quelque chose, quelqu'un peut éditer votre fonction, allouer quelque chose et avoir besoin de le nettoyer avant de revenir. Si cela se produit, ce sera un travail de cauchemar en parcourant toutes les déclarations.

En programmation C ++, vous avez des destructeurs et même maintenant des gardes de sortie. Tous ces éléments doivent être présents pour que le code soit protégé contre les exceptions en premier lieu. Le code est donc bien protégé contre une sortie anticipée. Par conséquent, cette opération ne présente aucun inconvénient logique et constitue uniquement un problème de style.

Je ne connais pas suffisamment Java pour savoir si le code de bloc sera finalement appelé et si les finaliseurs peuvent gérer le problème de la nécessité de s’assurer que quelque chose se passe.

C # Je ne peux certainement pas répondre à.

Le langage D vous donne les protections de sortie intégrées appropriées et est donc bien préparé pour une sortie anticipée et ne doit donc pas poser de problème autre que le style.

Les fonctions ne devraient évidemment pas être aussi longues en premier lieu, et si vous avez une énorme instruction switch, votre code est probablement aussi mal pris en compte.

Vache à lait
la source
1
C ++ autorise ce que l'on appelle l'optimisation des valeurs de retour, ce qui permet au compilateur d'omettre essentiellement l'opération de copie qui aurait normalement lieu lorsque vous renvoyez une valeur. Cependant, dans des conditions difficiles, il est difficile d’obtenir des résultats et la multiplicité des valeurs renvoyées. En d'autres termes, l'utilisation de plusieurs valeurs de retour en C ++ peut réellement ralentir votre code. Ceci est certainement valable pour MSC, et étant donné que ce serait très difficile (voire impossible) d'implémenter RVO avec plusieurs valeurs de retour possibles, c'est probablement un problème pour tous les compilateurs.
Eamon Nerbonne
1
En C, utilisez simplement gotoet, éventuellement, un retour en deux points. Exemple (formatage de code non possible dans les commentaires):foo() { init(); if (bad) goto err; bar(); if (bad) goto err; baz(); return 0; err: cleanup(); return 1; }
mirabilos
1
Au lieu d'un goto, je préfère la "méthode d'extraction". Lorsque vous pensez avoir besoin d'implémenter une variable de valeur de retour ou un goto, afin de vous assurer que votre code de nettoyage est toujours appelé, vous sentez qu'il faut que vous divisiez le code en plusieurs fonctions. Cela vous permet de simplifier votre code conditionnel complexe à l'aide de clauses de garde et d'autres techniques de renvoi anticipé, tout en garantissant que votre code de nettoyage est toujours exécuté.
BrandonLWhite
"quelqu'un pourrait éditer votre fonction" - c'est une explication tellement ridicule pour la prise de décision. Tout le monde peut faire quelque chose dans le futur. Cela ne signifie pas que vous devriez faire quelque chose de spécifique aujourd'hui pour empêcher quelqu'un de casser des choses à l'avenir.
Victor Yarema
Appelé rendant votre code maintenable. Dans le monde réel, le code est écrit à des fins commerciales et parfois, un développeur doit ultérieurement le modifier. À l'époque, j'ai écrit cette réponse bien que j'aie corrigé CURL pour introduire la mise en cache et rappeler le gâchis spaghetti, c'est-à-dire qu'il fallait vraiment réécrire correctement le C ++ moderne
CashCow
9

Retour rapide pour le gagnant. Ils peuvent sembler laids, mais beaucoup moins laids que les gros ifemballages, surtout s’il ya plusieurs conditions à vérifier.

STW
la source
9

J'utilise les deux.

Si DoSomethingest 3-5 lignes de code alors le code est beau en utilisant la première méthode de formatage.

Mais s'il a beaucoup plus de lignes que cela, alors je préfère le deuxième format. Je n'aime pas quand les crochets d'ouverture et de fermeture ne sont pas sur le même écran.

Azheglov
la source
2
Beau pas question! Trop d'indentation!
JimmyKane
@JimmyKane Il y a peu d'indentations qui peuvent se produire en 3 à 5 lignes, en particulier si vous avez besoin de 2 (?) Lignes par niveau, le reste est indenté: un pour la structure de contrôle et le début du bloc, un pour la fin du bloc.
Déduplicateur
8

Une raison classique pour une seule entrée, une seule sortie est que sinon, la sémantique formelle devient indiciblement laide (sinon, GOTO a été considéré comme nuisible).

En d'autres termes, il est plus facile de déterminer quand votre logiciel quittera la routine si vous n'avez qu'un seul retour. Ce qui est aussi un argument contre les exceptions.

En règle générale, je minimise l'approche de retour rapide.

Paul Nathan
la source
mais pour un outil d'analyse formel, vous pouvez synthétiser une fonction externe avec la sémantique dont il a besoin et garder le code lisible par l'homme.
Tim Williscroft
@Tim. Cela dépend de la quantité de trempage que vous voulez faire et de ce que vous analysez. Je trouve SESE assez lisible si le codeur était sain d'esprit.
Paul Nathan
Mon attitude envers l'analyse est façonnée par les optimisations du projet Self. 99% des appels de méthodes dynamiques peuvent être résolus et éliminés de manière statique. vous pouvez alors analyser du code en ligne droite. En tant que programmeur actif, je peux supposer de manière fiable que la plupart du code sur lequel je travaille a été écrit par des programmeurs moyens. Donc, ils n'écriront pas un très bon code.
Tim Williscroft
@ Tim: assez juste. Même si je me sens obligé de souligner que les langages statiques ont encore beaucoup de jeu.
Paul Nathan
Une raison qui ne tient pas face aux exceptions. C’est pourquoi une seule sortie est excellente en C, mais ne vous achète rien en C ++.
peterchen
7

Personnellement, je préfère effectuer des contrôles de réussite / d’échec au début. Cela me permet de regrouper la plupart des défaillances les plus courantes au sommet de la fonction avec le reste de la logique à suivre.

Nathan
la source
6

Ça dépend.

Retour anticipé s'il existe une impasse évidente à vérifier immédiatement qui rendrait inutile l'exécution du reste de la fonction. *

Définissez Retval + single return si la fonction est plus complexe et pourrait avoir plusieurs points de sortie sinon (problème de lisibilité).

* Cela peut souvent indiquer un problème de conception. Si vous constatez que bon nombre de vos méthodes doivent vérifier un état externe / paramater ou autre avant d'exécuter le reste du code, c'est probablement quelque chose qui devrait être traité par l'appelant.

Tables Bobby
la source
6
Quand j'écris du code qui peut être partagé, mon mantra est "Ne supposez rien. Ne faites confiance à personne." Vous devez toujours valider vos entrées et tout état externe dont vous dépendez. Mieux vaut jeter une exception que de corrompre quelque chose parce que quelqu'un vous a donné de mauvaises données.
TMN
@TMN: Bon point.
Bobby Tables
2
Le point clé ici est que dans OO, vous lancez une exception , pas de retour . Les retours multiples peuvent être mauvais, les lancements multiples d'exceptions ne sont pas nécessairement une odeur de code.
Michael K
2
@MichaelK: Une méthode devrait être une exception si la post-condition ne peut pas être remplie. Dans certains cas, une méthode doit quitter tôt car la post-condition a été atteinte avant même que la fonction ne soit lancée. Par exemple, si l’on invoque une méthode "Définir l’étiquette du contrôle" pour modifier l’étiquette en contrôle, l’étiquette de Fredla fenêtre l’est déjà Fred, et définir le nom du contrôle à son état actuel forcerait le redessin (ce qui, même s’il est potentiellement utile dans certains cas, ennuyeux dans l’actuel), il serait parfaitement raisonnable d’avoir la méthode set-name early-exit si les anciens et les nouveaux noms correspondent.
Supercat
3

Utilisez un si

Dans le livre de Don Knuth sur GOTO, je lis qu'il donne une raison pour que la condition la plus probable soit toujours la première dans une déclaration if. En supposant que cela reste une idée raisonnable (et non une idée par pure considération pour la rapidité de l'époque). Je dirais que les retours anticipés ne sont pas de bonnes pratiques de programmation, surtout si l'on considère qu'ils sont le plus souvent utilisés pour la gestion des erreurs, à moins que votre code ne risque pas d'échouer que de ne pas échouer :-)

Si vous suivez les conseils ci-dessus, vous aurez besoin de mettre ce retour au bas de la fonction, et vous pourriez tout aussi bien ne pas l'appeler retour, il suffit de définir le code d'erreur et de le renvoyer deux lignes. Réalisant ainsi l'idéal 1 entrée 1 sortie.

Delphi Spécifique ...

Je suis d’avis que c’est une bonne pratique de programmation pour les programmeurs Delphi, bien que je n’en ai aucune preuve. Pré-D2009, nous ne disposons pas d' une manière atomique pour renvoyer une valeur, nous avons exit;et result := foo;ou nous pourrions simplement jeter des exceptions.

Si vous deviez remplacer

if (true) {
 return foo;
} 

pour

if true then 
begin
  result := foo; 
  exit; 
end;

vous pourriez juste en avoir marre de voir cela au sommet de chacune de vos fonctions et préférer

if false then 
begin
  result := bar;

   ... 
end
else
   result := foo;

et juste éviter exitcomplètement.

Peter Turner
la source
2
Avec Delphis plus récent, il pourrait être abrégé. if true then Exit(foo);J'utilise souvent la technique pour initialiser resultà nilou FALSErespectivement, puis pour vérifier toutes les conditions d'erreur et juste Exit;si l'une d'entre elles est remplie. Le cas de réussite resultest alors (généralement) défini quelque part à la fin de la méthode.
jeudi
Oui, j'aime bien cette nouvelle fonctionnalité, même s'il semble que ce soit juste pour apaiser les programmeurs Java. Vous savez qu'ils nous laisseront définir nos variables dans les procédures.
Peter Turner
Et les programmeurs C #. Oui, pour être honnête, je trouve cela vraiment utile, car cela réduit le nombre de lignes entre déclaration et utilisation (l’IIRC a même quelques paramètres pour cela, oubliez le nom).
jeudi
2

Je suis d'accord avec l'énoncé suivant:

Personnellement, je suis un fan des clauses de garde (le deuxième exemple), car cela réduit l'indentation de la fonction. Certaines personnes ne les aiment pas car cela entraîne plusieurs points de retour de la fonction, mais je pense que c'est plus clair avec eux.

Tiré de cette question dans stackoverflow .

Toto
la source
+1 Pour les clauses de garde. Je préfère aussi un garde orienté positivement, par exemple: si (condition = false) retour plutôt que si (! Condition) retour
JustinC
1

J'utilise les retours anticipés presque exclusivement de nos jours, à l'extrême. J'écris ceci

self = [super init];

if (self != nil)
{
    // your code here
}

return self;

comme

self = [super init];
if (!self)
    return;

// your code here

return self;

mais ce n'est pas grave. Si vous avez plus d'un ou deux niveaux d'imbrication dans vos fonctions, ils doivent être supprimés.

Dan Rosenstark
la source
Je suis d'accord. L'indentation est ce qui cause des problèmes de lecture. Moins il y a d'indentation, mieux c'est. Votre exemple simple a le même niveau d'indentation, mais ce premier exemple passera sûrement à plus d'indentations nécessitant plus de puissance cérébrale.
user441521
1

Je préférerais écrire:

if(someCondition)
{
    SomeFunction();
}
Josh K
la source
2
eww. Est-ce que la pré-validation? Ou est-ce dans un mehtod dédié à la validation DoSomeFunctionIfSomeCondition?
STW
Comment cela maintient-il vos préoccupations séparées?
Juste
2
Vous violez l'encapsulation en rendant la mise en oeuvre de la fonction (sa logique de dépendance) externe.
TMN
1
Si cela est exécuté dans une méthode publique et que SomeFunction () est une méthode privée dans la même classe, cela pourrait aller. Mais si quelqu'un d'autre appelle SomeFunction (), vous devez dupliquer les contrôles. Je trouve qu'il est préférable de faire en sorte que chaque méthode prenne soin de ce dont elle a besoin pour faire son travail, personne d'autre ne devrait avoir à le savoir.
Par Wiklander
2
C’est définitivement le style proposé par Robert C. Martin dans «Clean Code». Une fonction ne devrait faire qu'une chose. Cependant, dans les "Schémas de mise en œuvre", suggère Kent Beck, la seconde est meilleure parmi les deux options proposées dans le PO.
Scott Whitlock
1

Comme vous, j’écris habituellement le premier mais préfère le dernier. Si j'ai beaucoup de contrôles imbriqués, je refactorise généralement à la deuxième méthode.

Je n'aime pas la façon dont le traitement des erreurs est éloigné du chèque.

if not error A
  if not error B
    if not error C
      // do something
    else handle error C
  else handle error B
else handle error A

Je préfère ça:

if error A
  handle error A; return
if error B
  handle error B; return
if error C
  handle error C; return

// do something
secousse
la source
0

Les conditions sur le dessus sont appelées "conditions préalables". En mettant if(!precond) return;, vous listez visuellement toutes les conditions préalables.

L'utilisation du grand bloc "if-else" peut augmenter le temps d'indentation (j'ai oublié la citation concernant les indentations à 3 niveaux).

Ming-Tang
la source
2
Quelle? Vous ne pouvez pas faire un retour rapide en Java? C #, VB (.NET et 6), et apparemment Java (ce que je supposais, mais que je devais rechercher car je n'ai pas utilisé le langage depuis 15 ans) permettent tous des retours rapides. alors n'accusez pas les 'langages fortement typés' de ne pas avoir cette fonctionnalité. stackoverflow.com/questions/884429/…
ps2goat
-1

Je préfère garder si les déclarations sont petites.

Alors, choisissez entre:

if condition:
   line1
   line2
    ...
   line-n

et

if not condition: return

line1
line2
 ...
line-n

Je choisirais ce que vous avez décrit comme un "retour anticipé".

Remarquez, je me fous des retours anticipés ou autres, j'aime vraiment simplifier le code, raccourcir le corps des déclarations if, etc.

Niché si et pour et tandis que sont horribles , évitez-les à tout prix.

gains
la source
-2

Comme d’autres disent, ça dépend. Pour les petites fonctions qui renvoient des valeurs, je peux coder les premiers retours. Mais pour les fonctions importantes, j'aime toujours avoir une place dans le code où je sais que je peux mettre quelque chose qui sera exécuté avant son retour.

Mike Dunlavey
la source
-2

Je pratique l'échec rapide au niveau de la fonction. Il garde le code cohérent et propre (pour moi et ceux avec qui j'ai travaillé). Pour cette raison, je reviens toujours tôt.

Pour certaines conditions souvent vérifiées, vous pouvez implémenter des aspects pour ces vérifications si vous utilisez AOP.

Steven Evers
la source