Répétition de code vs méthode multi-responsable

11

J'essaie de suivre le principe de responsabilité unique (PRS) et aussi d'omettre les répétitions de code. Cependant, il y a souvent des endroits où il y a des répétitions de code qui ne sont rien d'autre que des blocs de code d'invocations qui résistent à les extraire dans au moins une méthode nommée complète:

DoAction1();
DoAction2();

if (value)
    DoAction3();

DoAction4();

Quelle est la meilleure façon d'extraire un tel code dans une méthode et comment le nommer?

yBee
la source
1
Pourquoi résistent-ils à être extraits dans une autre méthode significative?
Chandranshu
Toutes les actions font quelque chose sans rapport. Je dois écrire: void MethodThatDoAction1ThenAction2AndAction3IfValueAndThenAction4(). Je préférerais voir plus de sens dans: CodeBlock1().
yBee
4
Ensuite, il s'agit simplement de nommer correctement la méthode extraite, non? Je crains qu'il n'y ait pas de solution universelle et vous devrez le faire au cas par cas. Si vous répétez exactement ce même morceau de code à plus d'un endroit, il y a forcément un sens à vos actions et vous devez regarder attentivement (peut être discuté) pour trouver ce sens et lui donner un nom.
Chandranshu
1
Si c'est une consolation, le gagnant du printemps pour les noms de classe longs AbstractInterruptibleBatchPreparedStatementSetterest un peu plus petit que votre méthode.
Chandranshu
1
@Chandranshu Je pense que vos commentaires fonctionneraient bien comme réponse à cette question.
Simon Forsberg

Réponses:

13

Je pense qu'il s'agit simplement de nommer correctement la méthode extraite. Je crains qu'il n'y ait pas de solution universelle et vous devrez le faire au cas par cas. Si vous répétez exactement ce même morceau de code à plus d'un endroit, il y a forcément un sens à vos actions et vous devez regarder attentivement (peut être discuté) pour trouver ce sens et lui donner un nom.

Si c'est une consolation, le gagnant du printemps pour les noms de classe longs est AbstractInterruptibleBatchPreparedStatementSetterde 49 caractères.

Chandranshu
la source
11

En fait, vous souffrez de la tâche la plus difficile à accomplir par un programmeur - nommer les choses . Désolé pour une réponse aussi amature mais je n'ai pas pu résister. Vous pouvez constater que beaucoup de gens en souffrent , même sur Internet, vous pouvez trouver ce type d'essai .

Maintenant, si votre code répétitif est tous ensemble, il en résulte un travail et ces méthodes ne sont pas utilisées par d'autres méthodes, alors elles sont simplement la méthode Helpers , puis la méthode résultante peut être nommée comme doXXXou makeXXX... Je pense que vous obtenez le point.

Comme Chandranshu l'a dit "Si vous répétez ce [...] sens et donnez-lui un nom". est au point et approprié.

entrez la description de l'image ici

Photo gracieuseté: CodeChef Facebook Page Photo

Anirban Nag 'tintinmj'
la source
2

Je pense que vous allez trop loin dans le principe de la répétition du code. Pensez au point d'éviter la répétition de code. Le but est de réduire la quantité de code qui doit être vérifiée en cas de changement de logique et d'augmenter la compréhension en factorisant des blocs visiblement similaires.

Les inconvénients de la factorisation afin d'éviter la répétition sont que si l'un des blocs partagés doit changer, vous avez maintenant besoin d'un héritage encore plus complexe ou d'une commutation entre l'implémentation standard et non standard.

Donc, pesez soigneusement la possibilité que la logique d'un seul de ces blocs change sans les autres par rapport aux avantages de compréhension obtenus en éliminant cette similitude. Si une implémentation pouvait se séparer des autres, il serait peut-être préférable de simplement répéter le code.

Tout en conservant ce code répété, à mesure qu'il devient plus complexe et que votre domaine problématique devient plus défini, vous pouvez alors trouver plus approprié de factoriser les sections répétées, désormais plus complexes, mais aussi plus définies.

J'essaie généralement de maintenir la même nature de l'éditeur de texte pendant un certain temps jusqu'à ce que je puisse voir si quelque chose qui semble répétitif s'avère utile. Je garde juste la répétition, mais je garde un œil sur l'avenir de ce bloc en le gardant textuellement facile à faire correspondre plus tard.

La plupart du temps, la même nature et l'éventuel affacturage commencent à se dissiper, en tant que règles commerciales réelles et capricieuses et en une logique très dépendante, souvent arbitraire; comme traiter les bizarreries de plusieurs implémentations de base de données courantes (ANSI_NULLS ou quelques-unes de celles-ci viennent à l'esprit) sont ajoutées; forçant ce qui semblait être une pure logique dans un gâchis tordu, essayant de fournir une logique de décision raisonnable et défendable face au désordre de l'état de l'industrie.

Il me semble que si les gens essayaient de prendre en compte ce que vous essayez de prendre en compte, nous aurions une bibliothèque entière de constructions sans valeur comme Do1Then2If2False Do1IfTrueDo2.

Il doit être plus complexe et plus clair que le bloc ne va pas changer pour justifier sa prise en compte.

C'est un logiciel . Vous pouvez revenir en arrière et modifier quelques blocs qui sont les mêmes en ce moment. Cela prendra 5 minutes. Et vous pouvez économiser des heures de perte d'affacturage, puis des heures supplémentaires d'héritage et de développement de commutation, simplement en le laissant et en vous assurant d'avoir un bon clavier anti-RSI.

Andyz Smith
la source
J'aime la partie qui extrait l'essence de la répétition de code: pour réduire la quantité de code qui doit être vérifiée lorsqu'il y a un changement de logique
yBee
1

Si vous suivez vraiment le principe de responsabilité unique, ce bloc de code doit avoir un objectif spécifique, non? Sinon, ces méthodes seraient dans des classes distinctes.

Alors, quel est le but de ce bloc de code? Déterminez cela et vous devriez pouvoir trouver un nom.

Si vous ne pouvez toujours pas trouver le nom de cette chose, alors peut-être que ces méthodes ne vont pas du tout ensemble. C'est-à-dire que ce bloc de classe / code a plus d'une responsabilité. Dans ce cas, une refactorisation pour diviser les tâches peut révéler un meilleur nom révélateur de but.

Allan
la source
1

J'ai eu une situation qui peut être similaire à la vôtre:

Il existe une classe qui définit la fonctionnalité de l'objet qu'il représente:

class Functionality
{
protected:
void functionA();
void functionB();
...
void functionZ();
}

Ensuite, il existe une classe qui définit les flux de travail pour les opérations de haut niveau que l'objet effectue:

class Workflows: private Functionality
{
    void WorkflowA()
    {
        functionA();

        if (m_bValue) {
            functionB();
        }

        functionC();
    }
    ...
    void WorkflowB();
}

Si vous êtes dans une situation similaire, identifiez ce que vos classes représentent (dans ce cas, fonctionnalité / workflows), puis nommez les méthodes en conséquence.

Avertissement: Les noms de classe utilisés dans cet exemple sont extrêmement inexacts, mais les noms de méthode donnent un indice. Discrétion conseillée.

dotbugfix
la source