Ligne supplémentaire dans le bloc par rapport au paramètre supplémentaire dans le code propre

33

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?

R. Schmitz
la source
58
Quel est le problème avec flurps.Add(CreateFlurp(badaBoom));?
cmaster
47
Non, c'est juste une seule déclaration. C'est juste une expression imbriquée trivialement (un seul niveau imbriqué). Et si un simple 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 plus sqrt(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.
cmaster
6
@cmaster Même l'assemblage x86 n'a pas strictement d'instructions à un seul opérateur. Les modes d'adressage mémoire comportent de nombreuses opérations compliquées et peuvent être utilisés pour l'arithmétique. En fait, vous pouvez créer un ordinateur complet de Turing en utilisant uniquement des movinstructions x86 et une seule jmp toStartà la fin. Quelqu'un a en fait créé un compilateur qui fait exactement cela: D
Luaan
5
@Luaan Sans parler de la tristement célèbre rlwimiinstruction 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 :-)
cmaster
7
@ R.Schmitz "Je fais de la programmation pour usage général" - en fait non, vous ne programmez que pour une utilisation spécifique (je ne sais pas dans quel but, mais je suppose que vous le faites ;-). Il existe littéralement des milliers d’objectifs pour la programmation, et les styles de codage optimaux varient, ce qui ne vous convient peut-être pas, et vice-versa: souvent, le conseil est absolu (" faites toujours X; Y est mauvais "etc) en ignorant que dans certains domaines, il est absolument impossible de s'y tenir. C'est pourquoi les conseils de livres tels que Clean Code doivent toujours être pris avec une pincée de sel (pratique) :)
psmears

Réponses:

104

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'appelle Select. Quelque chose comme ça:

public List<Flurp> CreateFlurps(List<BadaBoom> badaBooms)
{
    return badaBooms.Select(BadaBoom => CreateFlurp(badaBoom)).ToList();
}
Amon
la source
7
Le code est toujours faux et il réinvente inutilement la roue. Pourquoi appeler CreateFlurps(someList)alors que la BCL fournit déjà someList.ConvertAll(CreateFlurp)?
Ben Voigt
44
@ BenVoigt Ceci est une question au niveau de la conception. Je ne me soucie pas de la syntaxe exacte, d'autant plus qu'un tableau blanc n'a pas de compilateur (et j'ai écrit C # pour la dernière fois en 2009). Mon propos n'est pas «j'ai montré le meilleur code possible» mais «en passant, il s'agit d'un modèle commun qui est déjà résolu». Linq est un moyen de le faire, le ConvertAll en mentionne un autre . Merci de suggérer cette alternative.
amon
1
Votre réponse est raisonnable, mais le fait que LINQ détourne la logique et ramène l'énoncé à une seule ligne semble contredire votre conseil. En remarque, BadaBoom => CreateFlurp(badaBoom)est redondant; vous pouvez passer CreateFlurpdirectement à la fonction ( Select(CreateFlurp)). (Pour autant que je sache, cela a toujours été le cas.)
jpmc26
2
Notez que cela supprime entièrement le besoin de la méthode. Le nom CreateFlurpsest en réalité plus trompeur et plus difficile à comprendre que simplement voir badaBooms.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.
Carl Leth
1
@ R.Schmitz Ce n'est pas difficile à comprendre, mais c'est moins facile à comprendre que 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, mais badaBooms.Select(CreateFlurp)ne peut pas. C'est également trompeur parce que c'est à tort de demander un Listau lieu d'un IEnumerable.
Carl Leth
61

Le nombre idéal d'arguments pour une fonction est zéro (niladique)

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:

public List<Flurp> CreateFlurps(List<BadaBoom> badaBooms)
{
    List<Flurp> flurps = new List<Flurp>();
    foreach (BadaBoom badaBoom in badaBooms)
    {
        flurps.Add(CreateFlurp(badaBoom));
    }
    return flurps;
}

Mais c'est un code très long (C #). Faites-le simplement comme:

IEnumerable<Flurp> CreateFlurps(IEnumerable<BadaBoom> badaBooms) =>
    from badaBoom in babaBooms select CreateFlurp(badaBoom);
David Arno
la source
14
Une fonction avec zéro argument est censée impliquer que l’objet encapsule les données requises, et non pas que des objets existent dans un état global en dehors d’un objet.
Ryathal
19
@Ryathal, deux points: (1) si vous parlez de méthodes, pour la plupart des langages OO (tous?), Cet objet est déduit (ou explicitement indiqué, dans le cas de Python) en tant que premier paramètre. En Java, C #, etc., toutes les méthodes sont des fonctions avec au moins un paramètre. Le compilateur ne fait que cacher ce détail. (2) Je n'ai jamais parlé de "global". L'état de l'objet est externe à une méthode par exemple.
David Arno
17
Je suis à peu près sûr que, quand Oncle Bob a écrit "zéro", il voulait dire "zéro (sans compter)".
Doc Brown
26
@DocBrown, probablement parce qu'il est un grand partisan du mélange de l'état et de la fonctionnalité d'objets, ainsi, par "fonction", il fait probablement référence à des méthodes. Et je ne suis toujours pas d'accord avec lui. Il est de loin préférable de ne donner à une méthode que ce dont elle a besoin, plutôt que de la laisser fouiller dans l'objet pour obtenir ce qu'elle veut (c'est-à-dire qu'il est classique de "dire, ne demandez pas" en action).
David Arno
8
@AlessandroTeruzzi, l'idéal est un paramètre. Zéro c'est trop peu. C'est pourquoi, par exemple, les langages fonctionnels en adoptent un comme nombre de paramètres à des fins de currying (en fait, dans certains langages fonctionnels, toutes les fonctions ont exactement un paramètre: ni plus, ni moins). Curry avec zéro paramètre serait absurde. Affirmer que "l'idéal est aussi peu que possible, l'ergo zéro est le meilleur" est un exemple de réduction de l'absurde .
David Arno
19

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.

public List<Flurp> CreateFlurps(List<BadaBoom> badaBooms)
    {
        List<Flurp> flurps = new List<Flurp>();
        foreach (BadaBoom badaBoom in badaBooms)
        {
            flurps.Add(badaBoom .CreateFlurp());
            //or
            badaBoom.AddToListAsFlurp(flurps);
            //or
            flurps.Add(new Flurp(badaBoom));
            //or
            //make flurps a member of the class
            //use linq.Select()
            //etc
        }
        return flurps;
    }

ou

foreach(var flurp in ConvertToFlurps(badaBooms))...

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.

Ewan
la source
Peut-être que vous souhaitez modifier cette réponse pour la rendre plus claire? Ma question était de savoir si une chose l'emporte sur une autre sous Clean Code. Vous dites que tout est faux, puis vous décrivez l'une des options que j'ai données. Pour le moment, cette réponse semble indiquer que vous suivez un programme anti-Clean Code plutôt que d'essayer de répondre à la question.
R. Schmitz
désolé, j’ai interprété votre question comme suggérant que la première était la manière «normale», mais que vous étiez poussé dans la seconde. Je ne suis pas anti-clean code en général, mais cette citation est évidemment fausse
Ewan
19
@ R.Schmitz, j'ai moi-même lu "Clean Code" et je suis presque ce que dit ce livre. Cependant, en ce qui concerne la taille parfaite de la fonction, il s’agit tout simplement d’une erreur. Le seul effet est qu'il transforme le code spaghetti en code riz. Le lecteur est perdu dans la multitude de fonctions triviales qui ne produisent qu'un sens raisonnable lorsqu'elles sont vues ensemble. Les humains ont une capacité de mémoire de travail limitée et vous pouvez la surcharger avec des instructions ou des fonctions. Vous devez trouver un équilibre entre les deux si vous voulez être lisible. Évitez les extrêmes!
cmaster
@ cmaster La réponse n'était que les 2 premiers paragraphes lorsque j'ai écrit ce commentaire. C'est une meilleure façon de répondre maintenant.
R. Schmitz
7
franchement je préférais ma réponse plus courte. Il y a trop de discours diplomatique dans la plupart de ces réponses. Le conseil cité est totalement faux, nul besoin de spéculer sur «ce que cela signifie vraiment» ou de tourner en rond pour essayer de trouver une bonne interprétation.
Ewan
15

La deuxième est définitivement pire, comme CreateFlurpInListaccepte 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:

public List<Flurp> CreateFlurps(List<BadaBoom> badaBooms)
{
    return badaBooms.Select(CreateFlurp).ToList();
}

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.

Euphorique
la source
Je ne me plaindrais pas tellement de cette méthode "ne pas être pure et plus difficile à raisonner" (bien que cela soit vrai), mais de la considérer comme une méthode totalement inutile pour traiter un cas particulier. Que faire si je veux créer un Flurp autonome, un Flurp ajouté à un tableau, à un dictionnaire, un Flurp qui est ensuite recherché dans un dictionnaire et le Flurp correspondant supprimé, etc.? Avec le même argument, le code Flurp aurait également besoin de toutes ces méthodes.
gnasher729
10

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: Simple Flurp[], List<Flurp>, LinkedList<Flurp>, Dictionary<Key, Flurp>et ainsi de suite. Mais avec a CreateFlurpInList(BadaBoom, List<Flurp>), je reviens vers vous demain pour vous demander de CreateFlurpInBindingList(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

assez souvent je me trouve créer une liste d'une autre liste

C'est juste une question d'utilisation des outils disponibles. La version la plus courte, la plus efficace et la meilleure est:

var Flurps = badaBooms.ConvertAll(CreateFlurp);

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.

Ben Voigt
la source
Ne pas utiliser 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 qui ConvertAllsoit disponible ici est que l’opérateur demande par erreur une Listméthode - ce devrait être un fichier IEnumerable.
Carl Leth
6

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

flups = badaBooms.Select(bb => new Flurp(bb));

pourrait être une possibilité. Ou vous pourriez faire quelque chose comme

flups.Add(new Flurp(badaBoom))

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").

vous double
la source