2009-04-14 4 views
8

J'ai personnellement aucun problème avec le code suivantOù est le bon équilibre des prédicats dans une seule instruction if?

if (Object foo != null && !String.IsNullOrEmpty(foo["bar"])) 
{ 
    // do something 
} 

Parce que je pense que ce qui suit est trop bavard

if (Object foo != null) 
{ 
    if (!String.IsNullOrEmpty(foo["bar"])) 
    { 
     // do something 
    } 
} 

Mais je ne voudrais pas aller si loin avec ce point de vue se dire qu'il y avait 5 prédicats et j'ai dû envelopper le texte dans l'éditeur pour les voir tous en même temps. Y a-t-il une «ligne» logique que vous dessinez sur le nombre de prédicats que vous incluez dans une seule instruction if dans un sens similaire à dire que les méthodes ne devraient jamais plus de 7 paramètres

+0

Ne dites jamais jamais. Avez-vous déjà utilisé plus de 7 paramètres dans la famille des fonctions printf? – Les

Répondre

10

Je ne pense pas que réécrire if-instructions dans deux est le chemin, surtout si vous considérez que l'ajout d'une clause else à vos exemples donne des résultats différents. Si je voulais réduire le nombre de propositions atomiques dans l'état, je facteur certains qui ont du sens ensemble à une fonction de leur propre, comme:

if(won_jackpot(obj)) ... 
3

Non le nombre de prédicats mais la complexité compte. Tant que vous pouvez comprendre ce que le code fait avec un minimum d'effort, cela me semble correct.

Pour ajouter à cela, je ne changerais pas à multiple ifs mais j'ajoute une fonction pour les prédicats. Spécialement si le même nombre de prédicats est utilisé à plusieurs endroits.

1

Je ne pense pas qu'il y ait des règles, cependant:

  1. est suffisamment complexe que lorsque vous montrez à quelqu'un d'autre, ils luttent pour le comprendre?
  2. doit-il couvrir plusieurs lignes?
  3. Avez-vous des conditions répétées vérifiant plusieurs clauses 'if'? Ceci indiquerait une refactorisation requise dans une certaine méthode

Si l'une des conditions ci-dessus s'applique, je la retravaillerais.

9

Je pense que c'est un équilibre entre les différents types d'opérateurs et un formatage correct. Si tous les opérateurs sont les mêmes (tous « et » ou tout « ou »), alors vous pouvez probablement concaténer ensemble un nombre indéfini d'expressions sans perdre la compréhension:

if (something() && 
    something_else() && 
    so_on() && 
    so_forth() && 
    some_more_stuff() && 
    yada() && 
    yada() && 
    yada() && 
    newman() && 
    soup_nazi() && 
    etc()) 
    ... 
3

Cela dépend de ce que vous faites. La deuxième déclaration produit un résultat différent lorsque vous l'inversez. (ajouter un "pas" devant) et est une source très fréquente de bugs.

J'ai vu du code avec environ 20 prédicats qui fonctionne (ou au moins, fonctionne assez bien!) La règle que j'utilise, si elle ressemble à un dîner de chiens, je considère refactoring.

4

Je ne crois pas qu'il y ait un nombre magique. Si tous les prédicats ont un sens ensemble, alors je les rassemblerai. Cela peut impliquer de scinder l'instruction if sur deux lignes, mais je n'introduis en général jamais d'instructions supplémentaires si elles sont superflues. Mais si c'est particulièrement long, vous devriez vous demander si toutes les affirmations sont vraiment nécessaires. Vous pourriez peut-être filtrer certaines des valeurs plus tôt ou quelque chose comme ça. La plus grande préoccupation est la lisibilité. S'il est difficile à comprendre pour quelqu'un d'autre, vous devez refactoriser votre code. Mais séparer le code en deux instructions if différentes rend rarement le code plus lisible, il prend juste plus de lignes.

5

La mémoire à court terme a une capacité de sept éléments, donner ou prendre deux. Cela implique qu'une expression impliquant plus de cinq objets dissemblables peut nécessiter une pause et y réfléchir.

1

Ce problème avec de longues listes de conditions ne sont pas tant perte de lisibilité, mais la perte de testabilité. Surtout quand il s'agit de mauvais arguments, il est parfois plus facile de demander pardon que de demander la permission (voir par exemple réponse à this question). De cette façon, vous gardez votre code propre et testable, et soit corrigez l'appelant ou laissez-les gérer l'exception qui en résulte.

0

Tout dépend de ce qui doit être fait. Mais une chose que vous voudrez peut-être examiner qui fera certains algorithmes moins verbeux est une instruction switch:

switch (myVal) 
{ 
    case "isThis": 
    case "isThat": 
    case "something else": 
     doThis(); 
     break; 
    case "another": 
    case "yet another": 
    case "even another": 
     doThat(); 
     break; 
    case "another one": 
    case "more more": 
     doThisAgain(); 
     break; 
}

Faire cela aurait été assez bavard si les déclarations d'autre. Certains codes auront besoin de tonnes de déclarations if et else, d'autres pourront être condensées, etc. Il ne faut jamais sacrifier la qualité de l'exécution du code pour la simplicité de la source du code.

0

Quelle est la meilleure solution? Ni. La sémantique des deux est différente.

Je suis d'accord que le fractionnement rend le débogage plus facile, mais il en va de points d'arrêt conditionnels :)

Si c'est une simple combinaison de 'ET est ou OR est c'est plus de 3 essais, refactoriser.