Quel est le meilleur style de codage?

Question

Lors de la révision du code, un développeur senior a commenté une imbrication dans mon code. Il m'a suggéré de définir une valeur bool afin de ne jamais avoir plus d'un niveau d'imbrication. Je pense que mon code est plus lisible mais je veux avoir l'avis des autres développeurs à ce sujet. Quel est le meilleur style? Son aversion pour la nidification est-elle fondée?

Vous trouverez ci-dessous quelques exemples de code simplifié.

imbriqué:

If(condition1)
{
    If(condition2)
    {
        if(condition3)
        {
            return true;
        }
        else
        {
            log("condition3 failed");
        }
    else
    {
        log("condition2 failed")
    }
}
else
{
    log("condition1 failed")
}

return false;

ou

Bool Driven:

bool bRC = false;

bRC = (condition1);
if(brc)
{
    bRC = (condition2);
}
else
{
    log("condition1 failed");
    return false;
}

if(bRC)
{
    bRC = (condition3);
}
else
{
    log("condition2 failed");
    return false;
}

if(bRC)
{
    return true;
}
else
{
    log("condition3 failed");
    return false;
}

Answer 1

J'aime mieux le tien, mais je ferais probablement quelque chose comme:

if (condition1 && condition2 && condition3)
{
    return true;
}
else if (!condition1)
{
    log("condition1 failed");
}
else if (!condition2)
{
    log("condition2 failed");
}
else
{
    log("condition3 failed");
}
return false;

Si les conditions sont des expressions complexes, je pourrais éventuellement affecter les expressions à des variables portant le nom approprié avant d'évaluer les instructions if afin d'éviter de devoir recalculer les valeurs de chaque if.

Ceci suppose que le mode normal est que toutes les conditions sont vraies et que vous voulez donc avoir cette vérification en premier. Si le mode normal est qu'une ou plusieurs conditions sont fausses, alors je le réorganiserais et vérifierais chaque négation à son tour et renverrais simplement vrai si toutes les vérifications échouaient. Cela supprimerait également la nécessité pour les variables temporaires de remplacer les expressions complexes.

Answer 2

Si vous n'avez pas de règles idiotes concernant les points de retour multiples, je pense que c'est assez agréable (et quelqu'un d'autre aussi, mais ils ont supprimé leur réponse pour des raisons inconnues):

if(!condition1)
{
    log("condition1 failed");
    return false;
}

if(!condition2)
{
    log("condition2 failed");
    return false;
}

if(!condition3)
{
    log("condition3 failed");
    return false;
}

return true;

C’est peut-être une aversion pour le super imbrication égale, mais c’est certainement plus propre que sa merde qui stocke les conditions booléennes dans certaines valeurs. Cependant, cela peut être moins lisible dans le contexte: considérez si l’une des conditions était isreadable () . Il est plus clair de dire if (isreadable ()) car nous voulons savoir si quelque chose est lisible. if (! isreadable ()) suggère si nous voulons savoir si ce n'est pas lisible, ce qui n'est pas notre intention. Il est certes discutable qu'il puisse y avoir des situations où l'une est plus lisible / intuitive que l'autre, mais je suis moi-même un partisan de cette voie. Si quelqu'un s'accroche au retour, vous pouvez le faire:

if(!condition1)
    log("condition1 failed");

else if(!condition2)
    log("condition2 failed");

else if(!condition3)
    log("condition3 failed");

else
    return true;

return false;

Mais c'est plutôt sournois et moins "clair". à mon avis.

Answer 3

Je trouve personnellement que le code imbriqué est beaucoup plus facile à lire.

Answer 4

Je préfère généralement imbriqué pour mes conditions; Bien sûr, si mes conditions imbriquées sont indentées trop à droite, je dois commencer à me demander s’il existe une meilleure façon de faire ce que j’essaie de faire (refactorisation, refonte, etc.)

Answer 5

Semblable à la version imbriquée, mais beaucoup plus propre pour moi:

if not condition1:
    log("condition 1 failed")
else if not condition2:
    log("condition 2 failed")
else if not condition3:
    log("condition 3 failed")
else
    return true;
return false;

Notez que chaque condition est évaluée une fois.

Answer 6

Le deuxième style est absurdement verbeux: a-t-il vraiment suggéré exactement cela? Vous n'avez pas besoin de la plupart de ces instructions if , car il y a un return dans la partie else.

Avec l'exemple imbriqué, vous vous assurez de ne pas oublier d'inclure les éventuelles clauses else .

Aucune de ces solutions ne me semble satisfaisante.

Answer 7

Le comportement boolé est déroutant. L'imbrication est correcte si nécessaire, mais vous pouvez supprimer une partie de la profondeur d'imbrication en combinant les conditions dans une seule instruction ou en appelant une méthode permettant d'effectuer une partie de l'évaluation ultérieure.

Answer 8

Je pense que les deux manières sont possibles et ont leurs avantages et inconvénients. J'utiliserais le style bool-driven dans les cas où j'aurais une imbrication vraiment profonde (8 ou 10 ou quelque chose comme ça). Dans votre cas avec 3 niveaux, je choisirais votre style mais pour l'échantillon exact ci-dessus, j'irais comme ça:

void YourMethod(...)
{
  if (condition1 && condition2 && consition3)
    return true;

  if (!condition 1)
    log("condition 1 failed");

  if (!condition 2)
    log("condition 2 failed");

  if (!condition 3)
    log("condition 3 failed");

  return result;
}

... ou comme ça si vous préférez un seul point de sortie (comme je le fais) ...

void YourMethod(...)
{
  bool result = false;

  if (condition1 && condition2 && consition3)
  { 
    result = true;
  }
  else
  {
    if (!condition 1)
      log("condition 1 failed");

    if (!condition 2)
      log("condition 2 failed");

    if (!condition 3)
      log("condition 3 failed");
  }
  return result;
}

Ainsi, vous aurez toutes les conditions ayant échoué enregistrées lors de la première exécution. Dans votre exemple, vous n'obtiendrez qu'une seule condition ayant échoué, même s'il existe plusieurs conditions.

Answer 9

J'irais probablement avec

   if (!condition1)      log("condition 1 failed");
   else if (!condition2) log("condition 2 failed");
   else if (!condition3) log("condition 3 failed");
   // -------------------------------------------
   return condition1 && condition2 && condition3;

qui, à mon avis, est équivalent et beaucoup plus propre ...

En outre, une fois que le client a décidé que toutes les conditions devaient être évaluées et consignées en cas d'échec, et pas seulement la première, les opérations sont beaucoup plus faciles à modifier:

   if (!condition1) log("condition 1 failed");
   if (!condition2) log("condition 2 failed");
   if (!condition3) log("condition 3 failed");
   // -------------------------------------------
   return condition1 && condition2 && condition3;

Answer 10

Voici comment je le mettrais en œuvre, à condition que vos implémentations reflètent réellement le comportement souhaité.

if (!condition1) {
    log("condition1 failed");
    return false;
}
if (!condition2) {
    log("condition2 failed");
    return false;
}
if (!condition3) {
    log("condition3 failed");
    return false;
}
return true;

Cependant, je pense qu'il est plus probable que chaque condition ayant échoué soit consignée.

result = true;
if (!condition1) {
    log("condition1 failed");
    result = false;
}
if (!condition2) {
    log("condition2 failed");
    result = false;
}
if (!condition3) {
    log("condition3 failed");
    result = false;
}
return result;

Answer 11

Si la langue prend en charge la gestion des exceptions, voici ce qui suit:

try {
    if (!condition1) {
        throw "condition1 failed";
    }

    if (!condition2) {
        throw "condition2 failed";
    }

    if (!condition3) {
        throw "condition3 failed";
    }

    return true;

} catch (e) {
    log(e);
    return false;
}

EDIT De charles bretana: voir Utilisation des exceptions pour les flux de contrôle

Answer 12

Je n'aime pas de toute façon. Quand vous avez tellement de nids, quelque chose ne va pas. Dans le cas d'une validation de formulaire ou de quelque chose qui nécessite effectivement quelque chose comme ceci, essayez de trouver quelque chose de plus modulaire ou plus compact.

Un exemple serait un tableau contenant les conditions, à travers lequel vous irez de temps en temps, et imprimez / séparez si nécessaire.

Il y a trop d'implémentations en fonction de vos besoins. Créer un exemple de code serait donc inutile.

En règle générale, lorsque votre code semble trop compliqué, il est nul :). Essayez de repenser cela. Suivre les pratiques de codage la plupart du temps rend le code beaucoup plus esthétique et court; et évidemment, aussi plus intelligent.

Answer 13

Code doit reformuler le problème dans une langue donnée. Par conséquent, je maintiens que ces extraits peuvent être "meilleurs". Cela dépend du problème modélisé. Bien que je suppose, aucune des solutions ne sera parallèle au problème actuel. Si vous mettez des termes réels au lieu de condition1,2,3, cela pourrait complètement changer le "meilleur" code.
Je pense qu’il existe une meilleure façon (en 3D) d’écrire cela tous ensemble.

Answer 14

if( condition1 && condition2 && condition3 )
    return true;

log(String.Format("{0} failed", !condition1 ? "condition1" : (!condition2 ? "condition2" : "condition3")));
return false;

De cette façon, vous n’aurez pas à voir beaucoup de lignes de code juste pour vous connecter. Et si toutes vos conditions sont vraies, vous ne perdez pas de temps à les évaluer au cas où vous devriez vous connecter.