Inversez l'instruction «if» pour réduire l'imbrication

Lorsque j'ai exécuté ReSharper sur mon code, par exemple:

    if (some condition)
    {
        Some code...            
    }

ReSharper m'a donné l'avertissement ci-dessus (inverser l'instruction "if" pour réduire l'imbrication) et a suggéré la correction suivante:

   if (!some condition) return;
   Some code...

J'aimerais comprendre pourquoi c'est mieux. J'ai toujours pensé que l'utilisation de "return" au milieu d'une méthode posait problème, un peu comme "goto".

Un retour au milieu de la méthode n'est pas forcément mauvais. Il pourrait être préférable de revenir immédiatement si cela rend plus clair l'intention du code. Par exemple:

double getPayAmount() {
    double result;
    if (_isDead) result = deadAmount();
    else {
        if (_isSeparated) result = separatedAmount();
        else {
            if (_isRetired) result = retiredAmount();
            else result = normalPayAmount();
        };
    }
     return result;
};

Dans ce cas, si _isDeadc'est vrai, nous pouvons immédiatement sortir de la méthode. Il serait peut-être préférable de le structurer de cette façon à la place:

double getPayAmount() {
    if (_isDead)      return deadAmount();
    if (_isSeparated) return separatedAmount();
    if (_isRetired)   return retiredAmount();

    return normalPayAmount();
};   

J'ai choisi ce code dans le catalogue de refactoring . Ce refactoring spécifique est appelé: Remplacer le conditionnel imbriqué par des clauses de garde.

Ce n'est pas seulement esthétique , mais cela réduit également le niveau d'imbrication maximal à l'intérieur de la méthode. Ceci est généralement considéré comme un plus car il facilite la compréhension des méthodes (et en effet, de nombreux outils d' analyse statique en fournissent une mesure comme l'un des indicateurs de la qualité du code).

D'un autre côté, cela fait également que votre méthode a plusieurs points de sortie, ce qu'un autre groupe de personnes considère comme un non-non.

Personnellement, je suis d'accord avec ReSharper et le premier groupe (dans une langue qui a des exceptions, je trouve idiot de discuter de "plusieurs points de sortie"; presque tout peut être lancé, il y a donc de nombreux points de sortie potentiels dans toutes les méthodes).

En ce qui concerne les performances : les deux versions devraient être équivalentes (sinon au niveau IL, puis certainement après que la gigue est terminée avec le code) dans toutes les langues. En théorie, cela dépend du compilateur, mais pratiquement tout compilateur largement utilisé d'aujourd'hui est capable de gérer des cas d'optimisation de code beaucoup plus avancés que cela.

C'est un peu un argument religieux, mais je suis d'accord avec ReSharper que vous devriez préférer moins d'imbrication. Je crois que cela l'emporte sur les inconvénients d'avoir plusieurs chemins de retour à partir d'une fonction.

La raison principale d'avoir moins d'imbrication est d'améliorer la lisibilité et la maintenabilité du code . N'oubliez pas que de nombreux autres développeurs devront lire votre code à l'avenir, et que le code avec moins d'indentation est généralement beaucoup plus facile à lire.

Les conditions préalables sont un excellent exemple de cas où il est correct de revenir tôt au début de la fonction. Pourquoi la lisibilité du reste de la fonction devrait-elle être affectée par la présence d'un contrôle de précondition?

En ce qui concerne les points négatifs concernant le retour plusieurs fois d'une méthode - les débogueurs sont assez puissants maintenant, et il est très facile de savoir exactement où et quand une fonction particulière revient.

Avoir plusieurs retours dans une fonction n'affectera pas le travail du programmeur de maintenance.

Une mauvaise lisibilité du code le sera.

Comme d'autres l'ont mentionné, il ne devrait pas y avoir de perte de performances, mais il y a d'autres considérations. Mis à part ces préoccupations valables, cela peut également vous ouvrir à des pièges dans certaines circonstances. Supposons que vous ayez doubleplutôt affaire à un :

public void myfunction(double exampleParam){
    if(exampleParam > 0){
        //Body will *not* be executed if Double.IsNan(exampleParam)
    }
}

Comparez cela avec l' inversion apparemment équivalente:

public void myfunction(double exampleParam){
    if(exampleParam <= 0)
        return;
    //Body *will* be executed if Double.IsNan(exampleParam)
}

Donc, dans certaines circonstances, ce qui semble être correctement inversé ifpeut ne pas l'être.

L'idée de ne revenir qu'à la fin d'une fonction est revenue des jours avant que les langues ne prennent en charge les exceptions. Il permettait aux programmes de compter sur la possibilité de mettre du code de nettoyage à la fin d'une méthode, puis d'être sûr qu'il serait appelé et qu'un autre programmeur ne cacherait pas un retour dans la méthode qui aurait fait sauter le code de nettoyage . Un code de nettoyage ignoré peut entraîner une fuite de mémoire ou de ressources.

Cependant, dans un langage qui prend en charge les exceptions, il n'offre aucune garantie de ce type. Dans un langage qui prend en charge les exceptions, l'exécution de toute instruction ou expression peut provoquer un flux de contrôle qui entraîne la fin de la méthode. Cela signifie que le nettoyage doit être effectué en utilisant finalement ou en utilisant des mots clés.

Quoi qu'il en soit, je dis que je pense que beaucoup de gens citent la directive `` seul retour à la fin d'une méthode '' sans comprendre pourquoi cela a jamais été une bonne chose à faire, et que réduire l'imbrication pour améliorer la lisibilité est probablement un meilleur objectif.

Je voudrais ajouter qu'il y a un nom pour ceux qui sont inversés - Clause de garde. Je l'utilise chaque fois que je le peux.

Je déteste lire du code là où il y a si au début, deux écrans de code et pas d'autre. Inversez simplement si et revenez. De cette façon, personne ne perdra son temps à faire défiler.

http://c2.com/cgi/wiki?GuardClause

Cela n'affecte pas seulement l'esthétique, mais il empêche également l'imbrication de code.

Il peut en fait fonctionner comme une condition préalable pour garantir que vos données sont également valides.

C'est bien sûr subjectif, mais je pense que cela s'améliore fortement sur deux points:

• Il est maintenant immédiatement évident que votre fonction n'a plus rien à faire si condition est maintenue.
• Il maintient le niveau d'imbrication bas. L'imbrication nuit plus à la lisibilité que vous ne le pensez.

Plusieurs points de retour étaient un problème en C (et dans une moindre mesure en C ++) car ils vous obligeaient à dupliquer le code de nettoyage avant chacun des points de retour. Avec la collecte des ordures, le try| finallyconstruire et usingbloquer, il n'y a vraiment aucune raison pour que vous en ayez peur.

En fin de compte, cela revient à ce que vous et vos collègues trouvez plus facile à lire.

En termes de performances, il n'y aura pas de différence notable entre les deux approches.

Mais le codage ne se limite pas aux performances. La clarté et la maintenabilité sont également très importantes. Et, dans des cas comme celui-ci où cela n'affecte pas les performances, c'est la seule chose qui compte.

Il existe des écoles de pensée concurrentes quant à l'approche préférable.

Une vue est celle que d'autres ont mentionnée: la deuxième approche réduit le niveau d'imbrication, ce qui améliore la clarté du code. C'est naturel dans un style impératif: quand vous n'avez plus rien à faire, vous pouvez aussi bien revenir tôt.

Un autre point de vue, du point de vue d'un style plus fonctionnel, est qu'une méthode ne devrait avoir qu'un seul point de sortie. Tout dans un langage fonctionnel est une expression. Donc, si les instructions doivent toujours avoir une clause else. Sinon, l'expression if n'aurait pas toujours de valeur. Donc dans le style fonctionnel, la première approche est plus naturelle.

Les clauses de garde ou les conditions préalables (comme vous pouvez probablement le voir) vérifient si une certaine condition est remplie, puis interrompent le déroulement du programme. Ils sont parfaits pour les endroits où vous n'êtes vraiment intéressé que par un seul résultat d'une ifdéclaration. Alors plutôt que de dire:

if (something) {
    // a lot of indented code
}

Vous inversez la condition et rompez si cette condition inversée est remplie

if (!something) return false; // or another value to show your other code the function did not execute

// all the code from before, save a lot of tabs

returnest loin d'être aussi sale que goto. Il vous permet de passer une valeur pour montrer au reste de votre code que la fonction n'a pas pu s'exécuter.

Vous verrez les meilleurs exemples où cela peut être appliqué dans des conditions imbriquées:

if (something) {
    do-something();
    if (something-else) {
        do-another-thing();
    } else {
        do-something-else();
    }
}

contre:

if (!something) return;
do-something();

if (!something-else) return do-something-else();
do-another-thing();

Vous trouverez peu de gens qui soutiennent que le premier est plus propre mais bien sûr, c'est complètement subjectif. Certains programmeurs aiment savoir dans quelles conditions quelque chose fonctionne par indentation, alors que je préfère de loin garder un flux de méthode linéaire.

Je ne dirai pas un seul instant que les précons changeront votre vie ou vous rendront fou, mais vous pourriez trouver votre code juste un peu plus facile à lire.

Il y a plusieurs bons points ici, mais plusieurs points de retour peuvent également être illisibles , si la méthode est très longue. Cela étant dit, si vous allez utiliser plusieurs points de retour, assurez-vous simplement que votre méthode est courte, sinon le bonus de lisibilité de plusieurs points de retour pourrait être perdu.

La performance est en deux parties. Vous avez des performances lorsque le logiciel est en production, mais vous souhaitez également avoir des performances lors du développement et du débogage. La dernière chose qu'un développeur souhaite, c'est "d'attendre" quelque chose de trivial. En fin de compte, la compilation avec l'optimisation activée entraînera un code similaire. Il est donc bon de connaître ces petites astuces payantes dans les deux scénarios.

Le cas de la question est clair, ReSharper a raison. Plutôt que d'imbriquer des ifinstructions et de créer une nouvelle étendue dans le code, vous définissez une règle claire au début de votre méthode. Il augmente la lisibilité, il sera plus facile à maintenir et il réduit la quantité de règles à parcourir pour trouver où ils veulent aller.

Personnellement, je préfère un seul point de sortie. C'est facile à réaliser si vous gardez vos méthodes courtes et précises, et cela fournit un modèle prévisible pour la prochaine personne qui travaille sur votre code.

par exemple.

 bool PerformDefaultOperation()
 {
      bool succeeded = false;

      DataStructure defaultParameters;
      if ((defaultParameters = this.GetApplicationDefaults()) != null)
      {
           succeeded = this.DoSomething(defaultParameters);
      }

      return succeeded;
 }

Ceci est également très utile si vous souhaitez simplement vérifier les valeurs de certaines variables locales dans une fonction avant sa fermeture. Tout ce que vous devez faire est de placer un point d'arrêt sur le retour final et vous êtes assuré de l'atteindre (sauf si une exception est levée).

Beaucoup de bonnes raisons sur l'apparence du code . Mais qu'en est-il des résultats ?

Jetons un coup d'œil à du code C # et à sa forme compilée IL:

using System;

public class Test {
    public static void Main(string[] args) {
        if (args.Length == 0) return;
        if ((args.Length+2)/3 == 5) return;
        Console.WriteLine("hey!!!");
    }
}

Ce simple extrait peut être compilé. Vous pouvez ouvrir le fichier .exe généré avec ildasm et vérifier le résultat. Je ne publierai pas tout ce qui concerne l'assembleur mais je décrirai les résultats.

Le code IL généré effectue les opérations suivantes:

1. Si la première condition est fausse, passe au code où se trouve la seconde.
2. Si c'est vrai, saute à la dernière instruction. (Remarque: la dernière instruction est un retour).
3. Dans la deuxième condition, la même chose se produit après le calcul du résultat. Comparez et: arrivé à la Console.WriteLine si faux ou à la fin si c'est vrai.
4. Imprimez le message et revenez.

Il semble donc que le code ira jusqu'au bout. Et si nous faisons un if normal avec du code imbriqué?

using System;

public class Test {
    public static void Main(string[] args) {
        if (args.Length != 0 && (args.Length+2)/3 != 5) 
        {
            Console.WriteLine("hey!!!");
        }
    }
}

Les résultats sont assez similaires dans les instructions IL. La différence est qu'avant il devait y avoir des sauts par condition: si faux, passez au morceau de code suivant, si vrai, allez à la fin. Et maintenant, le code IL circule mieux et a 3 sauts (le compilateur l'a optimisé un peu): 1. Premier saut: lorsque la longueur est 0 à une partie où le code saute à nouveau (troisième saut) jusqu'à la fin. 2. Deuxième: au milieu de la deuxième condition pour éviter une instruction. 3. Troisièmement: si la deuxième condition est fausse, passez à la fin.

Quoi qu'il en soit, le compteur de programmes sautera toujours.

En théorie, l'inversion ifpourrait conduire à de meilleures performances si elle augmente le taux de succès de prédiction de branche. Dans la pratique, je pense qu'il est très difficile de savoir exactement comment se comportera la prédiction de branche, en particulier après la compilation, donc je ne le ferais pas dans mon développement quotidien, sauf si j'écris du code assembleur.

Plus d'informations sur la prédiction des branches ici .

C'est tout simplement controversé. Il n'y a pas "d'accord entre les programmeurs" sur la question du retour anticipé. C'est toujours subjectif, pour autant que je sache.

Il est possible de faire un argument de performance, car il vaut mieux avoir des conditions écrites pour qu'elles soient le plus souvent vraies; on peut également affirmer que c'est plus clair. En revanche, il crée des tests imbriqués.

Je ne pense pas que vous obtiendrez une réponse concluante à cette question.

Il y a déjà beaucoup de réponses perspicaces, mais je voudrais quand même m'orienter vers une situation légèrement différente: au lieu de la condition préalable, cela devrait être mis en tête d'une fonction, pensez à une initialisation pas à pas, où vous devez vérifier chaque étape pour réussir, puis passez à l'étape suivante. Dans ce cas, vous ne pouvez pas tout vérifier en haut.

J'ai trouvé mon code vraiment illisible lors de l'écriture d'une application hôte ASIO avec ASIOSDK de Steinberg, alors que je suivais le paradigme d'imbrication. Cela a atteint huit niveaux de profondeur, et je ne vois pas de défaut de conception, comme mentionné par Andrew Bullock ci-dessus. Bien sûr, j'aurais pu compresser du code interne dans une autre fonction, puis y imbriquer les niveaux restants pour le rendre plus lisible, mais cela me semble plutôt aléatoire.

En remplaçant l'imbrication par des clauses de garde, j'ai même découvert une méprise sur la partie du code de nettoyage qui aurait dû se produire beaucoup plus tôt dans la fonction plutôt qu'à la fin. Avec des branches imbriquées, je n'aurais jamais vu ça, on pourrait même dire qu'elles ont conduit à ma méprise.

Donc, cela pourrait être une autre situation où des if inversés peuvent contribuer à un code plus clair.

Éviter plusieurs points de sortie peut entraîner des gains de performances. Je ne suis pas sûr de C # mais en C ++, l'optimisation de la valeur de retour nommée (Copy Elision, ISO C ++ '03 12.8 / 15) dépend du fait d'avoir un seul point de sortie. Cette optimisation évite la copie de construire votre valeur de retour (dans votre exemple spécifique, cela n'a pas d'importance). Cela pourrait entraîner des gains de performances considérables dans les boucles serrées, car vous enregistrez un constructeur et un destructeur à chaque fois que la fonction est invoquée.

Mais pour 99% des cas, la sauvegarde des appels de constructeur et de destructeur supplémentaires ne vaut pas la perte de lisibilité des ifblocs imbriqués (comme d'autres l'ont souligné).

C'est une question d'opinion.

Mon approche normale consisterait à éviter les ifs à une seule ligne et à revenir au milieu d'une méthode.

Vous ne voudriez pas de lignes comme cela suggère partout dans votre méthode, mais il y a quelque chose à dire pour vérifier un tas d'hypothèses en haut de votre méthode, et ne faire votre travail réel que si elles réussissent toutes.

À mon avis, un retour précoce est bien si vous retournez simplement void (ou un code de retour inutile que vous ne vérifierez jamais) et cela pourrait améliorer la lisibilité car vous évitez l'imbrication et en même temps, vous expliquez que votre fonction est terminée.

Si vous renvoyez réellement une valeur de retour, l'imbrication est généralement une meilleure façon de procéder, car vous retournez votre valeur de retour juste au même endroit (à la fin - duh), et cela pourrait rendre votre code plus maintenable dans de nombreux cas.

Je ne suis pas sûr, mais je pense que R # essaie d'éviter les sauts lointains. Lorsque vous avez IF-ELSE, le compilateur fait quelque chose comme ceci:

Condition false -> far jump to false_condition_label

true_condition_label: instruction1 ... instruction_n

false_condition_label: instruction1 ... instruction_n

bloc d'extrémité

Si la condition est vraie, il n'y a pas de saut et pas de cache L1 de déploiement, mais le saut vers false_condition_label peut être très loin et le processeur doit déployer son propre cache. La synchronisation du cache coûte cher. R # essaie de remplacer les sauts lointains par des sauts courts et dans ce cas, il y a une plus grande probabilité que toutes les instructions soient déjà dans le cache.

Je pense que cela dépend de ce que vous préférez, comme mentionné, il n'y a pas d'accord général afaik. Pour réduire la gêne, vous pouvez réduire ce type d'avertissement à "Astuce"

Mon idée est que le retour "au milieu d'une fonction" ne devrait pas être aussi "subjectif". La raison est assez simple, prenez ce code:

    fonction do_something (data) {

      if (! is_valid_data (data)) 
            retour faux;


       do_something_that_take_an_hour (données);

       istance = new object_with_very_painful_constructor (données);

          si (la distance n'est pas valide) {
               Message d'erreur( );
                revenir ;

          }
       connect_to_database ();
       get_some_other_data ();
       revenir;
    }

Peut-être que le premier "retour" n'est pas si intuitif, mais c'est vraiment une économie. Il y a trop d'idées sur les codes propres, qui ont simplement besoin de plus de pratique pour perdre leurs mauvaises idées "subjectives".

Il existe plusieurs avantages à ce type de codage, mais pour moi, le gros avantage est que si vous pouvez revenir rapidement, vous pouvez améliorer la vitesse de votre application. IE Je sais qu'en raison de la condition X, je peux retourner rapidement avec une erreur. Cela supprime d'abord les cas d'erreur et réduit la complexité de votre code. Dans de nombreux cas, le pipeline du processeur peut désormais être plus propre, il peut arrêter les plantages ou les commutateurs du pipeline. Deuxièmement, si vous êtes dans une boucle, une rupture ou un retour rapide peut vous faire économiser beaucoup de CPU. Certains programmeurs utilisent des invariants de boucle pour effectuer ce type de sortie rapide, mais dans ce cas, vous pouvez casser votre pipeline de processeur et même créer un problème de recherche de mémoire et signifier que le processeur doit charger à partir du cache extérieur. Mais au fond, je pense que vous devriez faire ce que vous vouliez, c'est-à-dire que la boucle ou la fonction ne crée pas un chemin de code complexe juste pour implémenter une notion abstraite de code correct. Si le seul outil dont vous disposez est un marteau, tout ressemble à un clou.