Le contexte
Dans Clean Code , page 35, il est écrit
Cela implique que les blocs contenus dans les instructions if, les autres déclarations, les instructions while, etc., doivent comporter une ligne. Probablement cette ligne devrait être un appel de fonction. Non seulement cela garde la fonction englobante réduite, mais cela ajoute également une valeur documentaire car la fonction appelée dans le bloc peut avoir un nom joliment descriptif.
Je suis tout à fait d’accord, cela a beaucoup de sens.
Plus tard, à la page 40, il est dit à propos des arguments de la fonction
Le nombre idéal d'arguments pour une fonction est zéro (niladique). Vient ensuite l'un (monadique), suivi de près par deux (dyadique). Trois arguments (triadiques) doivent être évités autant que possible. Plus de trois (polyadiques) nécessitent une justification très spéciale - et ne devraient donc pas être utilisés de toute façon. Les arguments sont difficiles. Ils prennent beaucoup de pouvoir conceptuel.
Je suis tout à fait d’accord, cela a beaucoup de sens.
Problème
Cependant, assez souvent, je crée une liste à partir d'une autre liste et je vais devoir vivre avec l'un des deux maux.
Soit j'utilise deux lignes dans le bloc , une pour créer la chose, une pour l'ajouter au résultat:
public List<Flurp> CreateFlurps(List<BadaBoom> badaBooms)
{
List<Flurp> flurps = new List<Flurp>();
foreach (BadaBoom badaBoom in badaBooms)
{
Flurp flurp = CreateFlurp(badaBoom);
flurps.Add(flurp);
}
return flurps;
}
Ou j'ajoute un argument à la fonction pour la liste où la chose sera ajoutée, ce qui la rend "pire d'un argument".
public List<Flurp> CreateFlurps(List<BadaBoom> badaBooms)
{
List<Flurp> flurps = new List<Flurp>();
foreach (BadaBoom badaBoom in badaBooms)
{
CreateFlurpInList(badaBoom, flurps);
}
return flurps;
}
Question
Y a-t-il des (dés) avantages que je ne vois pas qui en rendent un préférable en général? Ou existe-t-il de tels avantages dans certaines situations? dans ce cas, que dois-je rechercher pour prendre une décision?
la source
flurps.Add(CreateFlurp(badaBoom));
?f(g(x))
est contre votre guide de style, eh bien, je ne peux pas réparer votre guide de style. Je veux dire, vous ne vous divisez pas non plussqrt(x*x + y*y)
en quatre lignes, n'est-ce pas ? Et cela fait trois (!) Sous-expressions imbriquées sur deux (!) Niveaux de nidification intérieurs (halètement!). Votre objectif doit être la lisibilité , pas les déclarations d'opérateur unique. Si vous voulez le plus tard, eh bien, j'ai le langage parfait pour vous: Assembler.mov
instructions x86 et une seulejmp toStart
à la fin. Quelqu'un a en fait créé un compilateur qui fait exactement cela: Drlwimi
instruction sur le PPC. (C’est l’acronyme de Rotate Left Word Immediate Mask Insert). Cette commande ne prenait pas moins de cinq opérandes (deux registres et trois valeurs immédiates) et elle effectuait les opérations suivantes: Le contenu du registre était pivoté par un décalage immédiat, un masque créé avec une seule séquence de 1 bits contrôlée par les deux autres opérandes immédiats, et les bits correspondant à 1 bit dans ce masque de l'autre opérande de registre ont été remplacés par les bits correspondants du registre pivoté. Instruction très cool :-)Réponses:
Ces lignes directrices sont une boussole, pas une carte. Ils vous orientent dans une direction raisonnable . Mais ils ne peuvent pas vraiment vous dire en termes absolus quelle solution est la «meilleure». À un moment donné, vous devez arrêter de marcher dans la direction indiquée par votre boussole, car vous êtes arrivé à destination.
Clean Code vous encourage à diviser votre code en très petits blocs évidents. C'est généralement une bonne direction. Mais, pris à l'extrême (comme le suggère une interprétation littérale des conseils cités), vous aurez alors subdivisé votre code en petits morceaux inutilement. Rien ne fait vraiment rien, tout est réservé aux délégués. Il s’agit essentiellement d’un autre type d’obscurcissement du code.
Il est de votre devoir de trouver un équilibre entre «plus petit, mieux c'est» et «trop petit, c'est inutile». Demandez-vous quelle solution est la plus simple. Pour moi, c’est clairement la première solution car elle constitue évidemment une liste. C'est un idiome bien compris. Il est possible de comprendre ce code sans avoir à regarder encore une autre fonction.
S'il est possible de faire mieux, c'est en notant que «transformer tous les éléments d'une liste en une autre liste» est un schéma courant qui peut souvent être extrait en utilisant une
map()
opération fonctionnelle . En C #, je pense que ça s'appelleSelect
. Quelque chose comme ça:la source
CreateFlurps(someList)
alors que la BCL fournit déjàsomeList.ConvertAll(CreateFlurp)
?BadaBoom => CreateFlurp(badaBoom)
est redondant; vous pouvez passerCreateFlurp
directement à la fonction (Select(CreateFlurp)
). (Pour autant que je sache, cela a toujours été le cas.)CreateFlurps
est en réalité plus trompeur et plus difficile à comprendre que simplement voirbadaBooms.Select(CreateFlurp)
. Ce dernier est complètement déclaratif - il n'y a pas de problème à décomposer et donc pas besoin de méthode du tout.badaBooms.Select(CreateFlurp)
. Vous créez une méthode pour que son nom (niveau élevé) remplace son implémentation (niveau bas). Dans ce cas, ils sont au même niveau, alors pour savoir exactement ce qui se passe, je dois simplement regarder la méthode (au lieu de la voir en ligne).CreateFlurps(badaBooms)
pourrait contenir des surprises, maisbadaBooms.Select(CreateFlurp)
ne peut pas. C'est également trompeur parce que c'est à tort de demander unList
au lieu d'unIEnumerable
.Non! Le nombre idéal d'arguments pour une fonction est un. S'il vaut zéro, vous garantissez que la fonction doit accéder à des informations externes pour pouvoir effectuer une action. "Oncle" Bob s'est très mal trompé.
En ce qui concerne votre code, votre premier exemple n'a que deux lignes dans le bloc car vous créez une variable locale sur la première ligne. Supprimez cette affectation et vous vous conformez à ces instructions de code propre:
Mais c'est un code très long (C #). Faites-le simplement comme:
la source
Le conseil «Clean Code» est complètement faux.
Utilisez deux lignes ou plus dans votre boucle. Cacher les deux mêmes lignes dans une fonction a du sens quand il s’agit de maths aléatoires qui nécessitent une description, mais cela ne fait rien quand les lignes sont déjà descriptives. 'Créer' et 'Ajouter'
La deuxième méthode que vous mentionnez n'a pas vraiment de sens, car vous n'êtes pas obligé d'ajouter un deuxième argument pour éviter les deux lignes.
ou
Comme l'ont noté d'autres personnes, l'avis selon lequel la meilleure fonction est une fonction sans arguments est au mieux biaisé au détriment de la programmation orientée objet et au pire au contraire d'un mauvais conseil.
la source
La deuxième est définitivement pire, comme
CreateFlurpInList
accepte la liste et la modifie, rendant la fonction plus simple et plus difficile à raisonner. Rien dans le nom de la méthode ne suggère que la méthode ne fait qu'ajouter à la liste.Et j'offre la troisième, meilleure option:
Et diable, vous pouvez intégrer cette méthode immédiatement s'il n'y a qu'un seul endroit où elle est utilisée, car one-liner est claire par elle-même et n'a donc pas besoin d'être encapsulée par méthode pour lui donner un sens.
la source
La version à un argument est préférable, mais pas principalement en raison du nombre d'arguments.
La raison la plus importante est qu'il a un couplage plus faible , ce qui le rend plus utile, plus facile à raisonner, plus facile à tester et moins susceptible de se transformer en clone copié-collé.
Si vous me fournissez un
CreateFlurp(BadaBoom)
, je peux l' utiliser avec tout type de conteneur de collection: SimpleFlurp[]
,List<Flurp>
,LinkedList<Flurp>
,Dictionary<Key, Flurp>
et ainsi de suite. Mais avec aCreateFlurpInList(BadaBoom, List<Flurp>)
, je reviens vers vous demain pour vous demander deCreateFlurpInBindingList(BadaBoom, BindingList<Flurp>)
sorte que mon modèle de vue puisse obtenir la notification du changement de la liste. Beurk!Un avantage supplémentaire est que la signature plus simple s’adapte mieux aux API existantes. Vous dites que vous avez un problème récurrent
C'est juste une question d'utilisation des outils disponibles. La version la plus courte, la plus efficace et la meilleure est:
Non seulement vous aurez moins de code pour écrire et tester, mais aussi plus rapide, car il
List<T>.ConvertAll()
est suffisamment intelligent pour savoir que le résultat aura le même nombre d’éléments que l’entrée et pour préallouer la liste de résultats à la taille appropriée. Alors que votre code (les deux versions) nécessitait de rallonger la liste.la source
List.ConvertAll
. Le moyen idiomatique de mapper un objet énumérable d'objets sur différents objets en C # est appeléSelect
. La seule raison quiConvertAll
soit disponible ici est que l’opérateur demande par erreur uneList
méthode - ce devrait être un fichierIEnumerable
.Gardez l’objectif général en tête: rendre le code facile à lire et à maintenir.
Souvent, il sera possible de regrouper plusieurs lignes dans une seule fonction significative. Faites-le dans ces cas. De temps en temps, vous devrez reconsidérer votre approche générale.
Par exemple, dans votre cas, remplacer l’implémentation complète par var
pourrait être une possibilité. Ou vous pourriez faire quelque chose comme
Parfois, la solution la plus propre et la plus lisible ne tient tout simplement pas dans une ligne. Donc, vous aurez deux lignes. Ne faites pas que le code soit plus difficile à comprendre, juste pour accomplir une règle arbitraire.
Votre deuxième exemple est (à mon avis) beaucoup plus difficile à comprendre que le premier. Ce n'est pas simplement que vous avez un deuxième paramètre, c'est que le paramètre est modifié par la fonction. Recherchez ce que Clean Code a à dire à ce sujet. (Vous n'avez pas le livre sous la main pour le moment, mais je suis presque sûr que c'est fondamentalement "ne faites pas cela si vous pouvez l'éviter").
la source