Lors d'une critique de code, un device senior a commenté certains nicheurs que j'avais assistés dans mon code. Il a suggéré de définir une valeur de bool pour que je n'ai jamais plus d'un niveau de nidification. Je pense que mon code est plus lisible mais je veux obtenir d'avis d'autres Devs à ce sujet. Quel est le meilleur style? Est-ce que son aversion contre les genoux à Nicher fondée?
Vous trouverez ci-dessous des exemples de code simplifiés.
niché: ou < p> bool axé:
14 Réponses :
Je préfère généralement imbriqué pour mes conditions; Bien sûr, si mes conditions imbriquées sont impressionnées trop loin à droite, je dois commencer à me demander s'il y a une meilleure façon d'aller sur ce que j'essaie de faire (refactoring, repensant, etc.)
Je trouve personnellement le code imbriqué de manière significative plus facile à lire.
Beaucoup plus facile de visualiser le flux de programme IMO.
Et je discuterais beaucoup plus facile de maintenir.
La personne imbriquée serait moins sujette d'erreur. Et vous réduisez la nécessité d'un booléen supplémentaire!
Et si l'imbrication commence à devenir trop, vous pouvez toujours décomposer le code dans plusieurs fonctions. Une quantité modérée de nidification est bien.
Vous aurez besoin d'un écran large, cependant. Ceci est particulièrement vrai si vous souhaitez également conserver une barre latérale visible, et si vous aimez la fonction longue et les noms variables.
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 Avec l'exemple imbriqué, vous comptez sur N'oubliez pas d'inclure d'autres clauses possibles aucun de ceux-ci ne me semble satisfaisant. si déclarations, car il existe un retour dans la partie d'autre.
sinon .
Je crains à chaque fois que je vois que "Bresult" anti-motif.
J'aime mieux le vôtre, mais je ferais probablement quelque chose comme: si les conditions sont des expressions complexes, je pourrais affecter les expressions à des variables nommées de manière appropriée avant d'évaluer les déclarations de si Évitez de devoir recomputer les valeurs de chaque si. Ceci suppose que le mode normal est que toutes les conditions sont vraies et que vous souhaitez donc avoir cette vérification en premier. Si le mode normal est qu'une ou plusieurs conditions sont fausses, je la réorganiserais et vérifiez chaque négation à tour de rôle et retournez simplement true si toutes les vérifications ont échoué. Cela supprimerait également la nécessité de variables temporaires à prendre la place des expressions complexes.
Je suis en faveur de cela aussi.
J'aime ça, mais si vous avez des fonctions dans la condition, vous pouvez appeler les fonctions à plusieurs fois (sauf si vous ne bénéficiez d'eval de court-circuit))
@ Mauris - Vrai, mais c'est probablement un cas où j'évaluerais la condition et que j'adressais à une variable locale pour éviter de le recalombrer.
+1 - Ceci clarifie le chemin d'exécution normal, ce qui facilite la visualisation et la maintenance du flux d'exécution que l'une des propositions intiales ( refactoring.com/catalog/... ).
Ce n'est pas équivalent et ne compilerait pas, car il ne renvoie pas faux si l'une des conditions est fausse. IT Eoulf a une erreur de compilation de "tous les chemins de code n'a pas un retour" (ou toutefois qui est libellé ..)
@Chals - que cela compilait ou non dépend entièrement de la langue / de la mise en œuvre dont vous parlez. En Java, non, ce ne serait pas. En C ou C ++, assurez-vous que cela le ferait.
@CHARDLES - Ce n'était qu'un extrait de code, pas la fonction entière de sorte que cela n'aurait donc pas été compilé. J'ai ajouté le retour quand même pour éviter toute confusion.
@ Mauris - Fait un bon point de stockage dans les variables locales facilite la débogage.
@Chris ... Ahh oui, dans C / C ++ Les types de retour de la fonction sont entièrement facultatifs, non? J'avais oublié ça - j'ai codé en C # depuis un moment ...
Le problème plus important avec ceci est que s'il existe des effets secondaires pendant que les conditions sont évaluées (elles sont des appels de fonction, par exemple), puis appelez des choses la deuxième fois un bogue.
@Jonathon - Ma réponse s'adresse à cela en notant que j'étais pré-évaluer les conditions et stockerais les valeurs dans des variables si les conditions étaient complexes (et j'envisagerais la fonction d'évaluation de la fonction une condition complexe). Bien entendu, cela introduirait également une autre question potentielle en ce que le code d'origine aurait pu s'appuyer sur l'évaluation paresseuse introduite par la nidification. Vous devriez envisager le code actuel en question lors de la refactorisation de savoir si ce flux de code serait équivalent et / ou approprié.
Personne n'a mentionné cela, mais ce qui est vraiment clair de cette façon, et c'est bien, c'est que le premier s'il contient le seul moyen de retourner vrai. Autre IFS Renvoie False indiquant une erreur et enregistre cette erreur. Je pense que c'est pourquoi il est plus facile de maintenir. Maintenant que j'y pense, en passant un pas en avant, si vous approvision, serait de convertir cela en classes d'Iévaluator. Chaque méthode d'évaluation serait chacune si je pense, bien que je pense que je connaisse des erreurs de journal dans cette méthode d'évaluation.
La manière battue de Bool est déroutante. La nidification est fine si nécessaire, mais vous pouvez enlever une partie de la profondeur de nidification en combinant les conditions dans une seule déclaration ou en appelant une méthode dans laquelle une autre de l'évaluation ultérieure est effectuée.
Le code doit reformuler le problème dans une langue donnée. Par conséquent, je maintiens que les deux extraits peuvent être "meilleurs". Cela dépend du problème de la modélisation. Bien que je suppose que l'autrement des solutions n'aura pas parallèle le problème réel. Si vous mettez des termes réels au lieu de condition1,2,3, cela pourrait changer complètement le "meilleur" code.
Je soupçonne qu'il y a une meilleure façon (3D) d'écrire cela ensemble.
Je pense que les deux voies sont possibles et ont leurs avantages et leurs inconvénients. J'utiliserais le style de bool dans des cas où j'aurais vraiment profil de nidification (8 ou 10 ou quelque chose comme ça). Dans votre cas avec 3 niveaux, je choisirais votre style mais pour l'échantillon exact d'en haut, j'irais comme ça: ... ou comme ça si vous préférez un seul point de sortie (Comme si je le fais) ... de cette façon, vous obtiendrez toute la condition faduite connectée à la première exécution. Dans votre exemple, vous n'obtiendrez qu'une seule condition panée enregistrée, même s'il y a plus d'une condition de défaillance.
Vérification redondante Les conditions à plusieurs reprises peuvent ne pas être favorables ou possibles; peut avoir des problèmes de performance ou des effets secondaires, respectivement.
J'irais probablement avec que je crois est équivalent et beaucoup plus propre ... En outre, une fois que le client décide que toutes les conditions devraient être évaluées et connecté s'ils échouent, pas seulement le premier qui échoue, il est beaucoup plus facile de modifier pour le faire:
Si la langue prend en charge la manipulation des exceptions, j'irais avec les éléments suivants:
try {
if (!condition1) {
throw "condition1 failed";
}
if (!condition2) {
throw "condition2 failed";
}
if (!condition3) {
throw "condition3 failed";
}
return true;
} catch (e) {
log(e);
return false;
}
-1: Stylistiquement mignon, mais une mauvaise idée pour de nombreuses langues. Par exemple, en Java, il est coûteux de lancer (en fait créer) une exception.
-1; Si c'est cher en Java, c'est Exorbitant dans Objective-C
Pourquoi supposez-vous que chaque peu de code devrait se soucier de la performance? Cela aurait facilement pu être une logique d'initialisation qui ne fonctionne qu'une seule fois au démarrage. Les initialisations complexes avec plusieurs points d'échec sont typiques de ces fonctions d'initialisation. Vous n'auriez généralement pas trois points d'échec dans une fonction de traitement des vecteurs critique de temps - à moins que vos vecteurs ne soient faux! :)
Honnêtement, j'aime bien le meilleur des réponses données.
Merci de déshabiller, mais nous semblons avoir des personnes heureuses-heureuses qui semblent être basses de la réponse sans considérer honnêtement le contexte dans lequel ce style serait utilisé.
+1 Stylistiquement j'aime ça parce que cela bouge de la journalisation au bloc de capture. Mais comme vous l'avez dit, son utilisation dépend des circonstances; Je ne voudrais pas l'utiliser pour la validation de formule où les conditions sont régulièrement faussement.
Je l'ai DV'D parce que jetant des exceptions comme une alternative au flux de contrôle est un anti-motif accepté, à éviter. Même si le code ne fonctionne que par une année ...
@CHARLES BRETANA, je serais intéressé à voir des citations pour cet anti-motif. Google n'a pas facilement donné quelque chose, mais je continuerai à regarder.
@Ates, cochez Google.com/...
Si vous n'avez pas de règles idiotes sur plusieurs points de retour, je pense que c'est assez agréable (et aussi quelqu'un d'autre, mais ils ont supprimé leur réponse pour des raisons inconnues):
if(!condition1)
log("condition1 failed");
else if(!condition2)
log("condition2 failed");
else if(!condition3)
log("condition3 failed");
else
return true;
return false;
Une variante mineure sur votre dernière variante: int rc = faux; si est comme avant; sinon rc = vrai; La fin de la fonction retourne RC. Rien de sournois; La fonction échoue jusqu'à ce que tout va bien.
C'est beaucoup mieux que l'approche booléenne, que j'appellerais une programmation de Cargo-culte .. Réagissez à la preuve superficielle de la complexité, mais mettant en œuvre une alternative tout aussi complexe mais plus obscure qui remplace l'imbrication de quelque chose de pire.
@Jonathan - J'envisagerai presque que cela soit plus sourné d'utiliser une variable inutile, mais ce n'est probablement pas important.
Voici comment je voudrais le mettre en œuvre, à condition que vos implémentations reflètent réellement le comportement souhaité. Cependant, je pense qu'il est plus probable que chaque condition fade doit être enregistrée.
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;
+1 Ceci est équivalent à la logique originale et sensiblement erser
Je viens de taper cela dans! Je suppose que je n'ai pas besoin de le poster. +1
Je n'aime pas d'une autre manière. Quand tu as tellement de nids, quelque chose ne va pas. Dans le cas d'une validation de formulaire ou quelque chose qui nécessite effectivement quelque chose comme ça, essayez de comprendre quelque chose plus modulaire ou plus compact.
Un exemple serait un tableau qui détient les conditions, à travers laquelle vous iriez itérer avec un moment et imprimer / casser au besoin.
Il y a trop d'implémentations en fonction de vos besoins afin de créer un exemple de code serait inutile.
En règle générale, lorsque votre code a l'air trop compliqué, ça craint :). Essayez de repenser. Après des pratiques de codage, la plupart des temps rend le code se révèle beaucoup plus esthétique et court; et évidemment, aussi plus intelligent.
if( condition1 && condition2 && condition3 )
return true;
log(String.Format("{0} failed", !condition1 ? "condition1" : (!condition2 ? "condition2" : "condition3")));
return false;
That way, you don't have to see many lines of code just for logging. And if all your conditions are true, you don't waste time evaluating them in case you have to log.
Ne blâmez pas votre Dev ;-)
Je suppose que je devrais voter pour l'option 2 puisque vous obtiendriez une erreur de compilation avec # 1
Veuillez indiquer la réaction de votre senior Dev quand vous leur montrez ce fil :)
Bonne question, beaucoup de discussions!
La version imbriquée n'est pas si agréable - mais la version booléenne est horrible.