Meilleure pratique si / retour

Je veux savoir ce qui est considéré comme le meilleur moyen de revenir quand j'ai une ifdéclaration.

Exemple 1:

public bool MyFunction()
{
   // Get some string for this example
   string myString = GetString();

   if (myString == null)
   {
      return false;
   }
   else
   {
      myString = "Name " + myString;
      // Do something more here...
      return true;
   }
}

Exemple 2:

public bool MyFunction()
{
   // Get some string for this example
   string myString = GetString();

   if (myString == null)
   {
      return false;
   }

   myString = "Name " + myString;
   // Do something more here...
   return true;
}

Comme vous pouvez le voir dans les deux exemples, la fonction reviendra, true/falsemais est-ce une bonne idée de mettre une elsedéclaration comme dans le premier exemple ou mieux de ne pas la mettre?

L'exemple 2 est connu sous le nom de bloc de garde. Il est plus approprié de renvoyer / jeter une exception tôt si quelque chose ne va pas (paramètre incorrect ou état non valide). En logique normale, il est préférable d'utiliser l'exemple 1

Mon style personnel consiste à utiliser le simple ifpour les blocs de garde et le if/ elsedans le code de traitement de la méthode.

Dans ce cas, vous utilisez la myString == nullcondition de garde, j'aurais donc tendance à utiliser le ifmotif simple .

Considérons un code un peu plus compliqué:

Exemple 1:

public bool MyFunction(myString: string){

    //guard block
    if (myString == null){
        return false;
    }
    else{
        //processing block
        myString = escapedString(myString);

        if (myString == "foo"){
            //some processing here
            return false;
        }
        else{
            myString = "Name " + myString;
            //other stuff
            return true;
        }
    }
}

Exemple 2:

public bool MyFunction(myString: string){

    //guard block
    if (myString == null){
        return false;
    }

    //processing block
    myString = escapedString(myString);

    if (myString == "foo"){
        //some processing here
        return false;
    }
    else{
        myString = "Name " + myString;
        //other stuff
        return true;
    }
}

Dans l'exemple 1, la protection et le reste de la méthode sont sous la forme if/ else. Comparez cela à l'exemple 2, où le bloc de garde est sous la ifforme unique , alors que le reste de la méthode utilise la forme if/ else. Personnellement, je trouve que l’exemple 2 est plus facile à comprendre, tandis que l’exemple 1 semble désordonné et trop en retrait.

Notez qu'il s'agit d'un exemple artificiel et que vous pouvez utiliser des else ifinstructions pour le nettoyer, mais mon objectif est de montrer la différence entre les blocs de garde et le code de traitement de la fonction.

Un compilateur décent devrait quand même générer la même sortie pour les deux. La seule raison d'utiliser l'un ou l'autre est la préférence personnelle ou de se conformer au style du code existant.

Personnellement, je préfère la deuxième méthode. J'ai l'impression que c'est plus court, moins indenté et plus facile à lire.

Ma pratique personnelle est la suivante:

• Je n’aime pas les fonctions avec plusieurs points de sortie, j’ai eu du mal à maintenir et à suivre, les modifications de code cassent parfois la logique interne car elle est intrinsèquement un peu bâclée. Lorsqu'il s'agit d'un calcul complexe, je crée une valeur de retour au début et la retourne à la fin. Cela me force à suivre attentivement chacun des chemins if-else, switch, etc., et à définir correctement la valeur aux emplacements appropriés. Je passe également un peu de temps à décider de définir une valeur de retour par défaut ou de la laisser non initialisée au début. Cette méthode est également utile lorsque la logique ou le type ou la signification de la valeur de retour change.

Par exemple:

public bool myFunction()
{
   // First parameter loading
   String myString = getString();

   // Location of "quick exits", see the second example
   // ...

   // declaration of external resources that MUST be released whatever happens
   // ...

   // the return variable (should think about giving it a default value or not) 
   // if you have no default value, declare it final! You will get compiler 
   // error when you try to set it multiple times or leave uninitialized!
   bool didSomething = false;

   try {
     if (myString != null)
     {
       myString = "Name " + myString;
       // Do something more here...

       didSomething = true;
     } else {
       // get other parameters and data
       if ( other conditions apply ) {
         // do something else
         didSomething = true;
       }
     }

     // Edit: previously forgot the most important advantage of this version
     // *** HOUSEKEEPING!!! ***

   } finally {

     // this is the common place to release all resources, reset all state variables

     // Yes, if you use try-finally, you will get here from any internal returns too.
     // As I said, it is only my taste that I like to have one, straightforward path 
     // leading here, and this works even if you don't use the try-finally version.

   }

   return didSomething;
}

• La seule exception: "sortie rapide" au début (ou dans de rares cas, dans le processus). Si la logique de calcul réelle ne peut pas gérer une certaine combinaison de paramètres d'entrée et d'états internes, ou a une solution simple sans exécuter l'algorithme, l'ajout de tout le code dans des blocs (parfois profonds) n'aiderait en rien. Il s'agit d'un "état exceptionnel", qui ne fait pas partie de la logique de base. Je dois donc sortir du calcul dès que j'ai détecté. Dans ce cas, il n'y a pas d'autre branche, dans des conditions normales, l'exécution continue. (Bien sûr, l’expression «état exceptionnel» s’exprime mieux en lançant une exception, mais parfois c’est exagéré.)

Par exemple:

public bool myFunction()
{
   String myString = getString();

   if (null == myString)
   {
     // there is nothing to do if myString is null
     return false;
   } 

   myString = "Name " + myString;
   // Do something more here...

   // not using return value variable now, because the operation is straightforward.
   // if the operation is complex, use the variable as the previous example.

   return true;
}

La règle "une sortie" est également utile lorsque le calcul nécessite des ressources externes que vous devez libérer, ou indique que vous devez réinitialiser avant de quitter la fonction; parfois, ils sont ajoutés plus tard au cours du développement. Avec plusieurs sorties à l'intérieur de l'algorithme, il est beaucoup plus difficile d'étendre correctement toutes les branches. (Et si des exceptions peuvent se produire, la libération / réinitialisation doit également être bloquée, pour éviter les effets secondaires dans de rares cas exceptionnels ...).

Votre cas semble tomber dans la catégorie "sortie rapide avant le vrai travail", et je l'écrirais comme votre version de l'exemple 2.

Je préfère utiliser des gardes partout où je peux pour deux raisons:

1. Ils permettent une sortie rapide en fonction de certaines conditions.
2. Supprimez la nécessité de déclarations if complexes et non nécessaires ultérieurement dans le code.

De manière générale, je préfère voir des méthodes où la fonctionnalité principale de la méthode est claire et minimale. Les blocs de garde aident à rendre cela visuel.

J'aime l'approche "Fall Through":

public bool MyFunction()
{
   string myString = GetString();

   if (myString != null)
   {
     myString = "Name " + myString;
     return true;
    }
    return false;
}

L'action a une condition spécifique, tout le reste n'est que le retour par défaut «échec».

Si j'ai une seule condition, je ne passerais pas trop de temps à méditer sur le style. Mais si j'ai plusieurs conditions de garde, je préférerais style2

Picuture cela. Supposons que les tests sont complexes et que vous ne voulez vraiment pas les lier dans une seule condition if-ORed pour éviter toute complexité:

//Style1
if (this1 != Right)
{ 
    return;
}
else if(this2 != right2)
{
    return;
}
else if(this3 != right2)
{
    return;
}
else
{
    //everything is right
    //do something
    return;
}

contre

//Style 2
if (this1 != Right)
{ 
   return;
}
if(this2 != right2)
{
    return;
}
if(this3 != right2)
{
    return;
}


//everything is right
//do something
return;

Ici, il y a deux avantages principaux

1. Vous séparez le code d’une seule fonction en deux blocs visuels: un bloc supérieur de validations (conditions de garde) et un bloc inférieur de code exécutable.

2. Si vous devez ajouter / supprimer une condition, vous réduisez vos chances de tout gâcher dans l’échelle if-elseif-else.

Un autre avantage mineur est que vous avez une paire d’accolades de moins à entretenir.

Cela devrait être une question qui sonne mieux.

If condition
  do something
else
  do somethingelse

s'exprime mieux alors

if condition
  do something
do somethingelse

pour les petites méthodes, cela ne fera pas beaucoup de différence, mais pour les grandes méthodes composées, cela peut rendre la compréhension plus difficile, car elle ne sera pas correctement séparée

Je trouve l'exemple 1 irritant, car l'instruction return manquante à la fin d'une fonction renvoyant une valeur déclenche immédiatement le signe "Attendez, il y a quelque chose qui ne va pas". Donc, dans des cas comme celui-ci, je choisirais l'exemple 2.

Cependant, il y a généralement plus d'implication, en fonction du but de la fonction, par exemple la gestion des erreurs, la journalisation, etc. Je suis donc avec la réponse de Lorand Kedves à ce sujet, et j'ai généralement un point de sortie à la fin, même au prix coûtant. d'une variable de drapeau supplémentaire. Cela facilite la maintenance et les extensions ultérieures dans la plupart des cas.

Lorsque votre ifinstruction revient toujours, il n'y a aucune raison d'utiliser une autre option pour le code restant dans la fonction. Cela ajoute des lignes et une indentation supplémentaires. L'ajout de code inutile rend la lecture plus difficile et, comme tout le monde le sait, la lecture du code est difficile .

Dans votre exemple, le elseest évidemment inutile, MAIS ...

Lorsque vous parcourez rapidement les lignes de code, vos yeux se penchent généralement sur les accolades et les empreintes pour vous faire une idée du flux de code. avant qu'ils ne s'installent réellement sur le code lui-même. Par conséquent, écrire le else, et mettre des accolades et une indentation autour du second bloc de code permet au lecteur de voir plus rapidement qu'il s'agit d'une situation "A ou B", dans laquelle l'un ou l'autre bloc sera exécuté. C'est-à-dire que le lecteur voit les accolades et l'indentation AVANT de voir returnaprès l'initiale if.

À mon avis, il s’agit d’un des rares cas où l’ajout d’un élément redondant au code le rend PLUS lisible, pas moins.

Je préférerais l'exemple 2, car il est immédiatement évident que la fonction retourne quelque chose . Mais même plus que cela, je préférerais revenir d'un endroit, comme ceci:

public bool MyFunction()
{
    bool result = false;

    string myString = GetString();

    if (myString != nil) {
        myString = "Name " + myString;

        result = true;
    }

    return result;
}

Avec ce style je peux:

1. Voyez tout de suite que je retourne quelque chose.

2. Saisir le résultat de chaque appel de la fonction avec un seul point d'arrêt.

Mon style personnel a tendance à aller

function doQuery(string) {
    if (!query(string)) {
        query("ABORT");
        return false;
    } // else
    if(!anotherquery(string) {
        query("ABORT");
        return false;
    } // else
    return true;
}

Utilisez les elseinstructions commentées pour indiquer le déroulement du programme et le garder lisible, tout en évitant les retraits massifs qui peuvent facilement atteindre l'écran si de nombreuses étapes sont impliquées.

Je souhaiterai écrire

public bool SetUserId(int id)
{
   // Get user name from id
   string userName = GetNameById(id);

   if (userName != null)
   {
       // update local ID and userName
       _id = id;
       _userNameLabelText = "Name: " + userName;
   }

   return userName != null;
}

Je préfère ce style car il est évident que j'y retournerais userName != null.

Ma règle générale est soit de faire un if-return sans autre (surtout des erreurs), soit de faire une chaîne complète if-else-if sur toutes les possibilités.

De cette façon, j'évite d'essayer d'être trop sophistiqué avec mes branches, car chaque fois que je finis par le faire, je code la logique d'application dans mes ifs, ce qui les rend plus difficiles à gérer (par exemple, si un enum OK ou ERR et que j'écris tous mes profitant du fait que! OK <-> ERR, il devient très pénible d’ajouter une troisième option à l’énum prochain.)

Dans votre cas, par exemple, j'utiliserais un simple if, puisque "return if null" est sûr d'être un modèle courant et qu'il est peu probable que vous ayez à vous soucier d'une troisième possibilité (en plus de null / non null) A l'avenir.

Cependant, si le test était quelque chose qui a fait une inspection plus approfondie sur les données, je serais plutôt orienté vers une analyse complète si-sinon.

Je pense que l'exemple 2 présente de nombreux pièges qui pourraient éventuellement conduire à un code non souhaité. Le premier objectif est basé sur la logique entourant la variable "myString". Par conséquent, pour être explicite, tous les tests de conditions doivent avoir lieu dans un bloc de code tenant compte de la logique connue et par défaut / inconnue .

Et si par la suite le code était introduit involontairement dans l'exemple 2 qui modifiait la sortie de manière significative:

   if (myString == null)
   {
      return false;
   }

   //add some v1 update code here...
   myString = "And the winner is: ";
   //add some v2 update code here...
   //buried in v2 updates the following line was added
   myString = null;
   //add some v3 update code here...
   //Well technically this should not be hit because myString = null
   //but we already passed that logic
   myString = "Name " + myString;
   // Do something more here...
   return true;

Je pense qu'avec le elsebloc qui suivait immédiatement la recherche de null, les programmeurs ayant ajouté les améliorations aux versions futures ajouteraient toute la logique, car nous avons maintenant une chaîne de logique qui n'était pas voulue pour la règle d'origine (retour si la valeur est nul).

Je crois profondément en cela à certaines des directives C # sur Codeplex (lien vers celui-ci ici: http://csharpguidelines.codeplex.com/ ) qui indiquent ce qui suit:

"Ajoute un commentaire descriptif si le bloc par défaut (else) est supposé être vide. De plus, si ce bloc n’est pas censé être atteint, lève une exception InvalidOperationException pour détecter les modifications futures susceptibles de tomber dans les cas existants. Cela garantit un meilleur code, car tous les chemins que le code peut emprunter ont été pensés. "

Je pense que la bonne pratique de programmation lors de l’utilisation de blocs logiques comme celui-ci consiste à toujours ajouter un bloc par défaut (if-else, case: default) afin de comptabiliser explicitement tous les chemins de code et de ne pas laisser le code ouvert à des conséquences logiques non voulues.