Principe de responsabilité unique - Comment puis-je éviter la fragmentation du code?

57

Je travaille dans une équipe où le chef d’équipe est un ardent défenseur des principes de développement SOLID. Cependant, il manque beaucoup d’expérience pour mettre au point des logiciels complexes.

Nous sommes dans une situation où il a appliqué SRP à ce qui était déjà une base de code assez complexe, qui est maintenant devenue très fragmentée et difficile à comprendre et à déboguer.

Nous avons maintenant un problème non seulement avec la fragmentation du code, mais aussi avec l'encapsulation, car les méthodes au sein d'une classe qui peut avoir été privée ou protégée ont été jugées représenter une "raison de changer" et ont été extraites dans des classes et interfaces publiques ou internes qui n'est pas conforme aux objectifs d'encapsulation de l'application.

Certains constructeurs de classe prennent en charge 20 paramètres d'interface. Notre enregistrement et notre résolution IoC deviennent donc un monstre à part entière.

Je veux savoir s’il existe une approche de type «refactor from SRP» que nous pourrions utiliser pour résoudre certains de ces problèmes. J'ai lu que cela ne violait pas SOLID si je créais un certain nombre de classes vides à grain plus grossier qui "encapsulent" un certain nombre de classes étroitement liées pour fournir un point d'accès unique à la somme de leurs fonctionnalités (c'est-à-dire imitant un trop d'implémentation de classe SRP)).

En dehors de cela, je ne peux pas penser à une solution qui nous permettra de poursuivre nos efforts de développement de manière pragmatique, tout en satisfaisant tout le monde.

Aucune suggestion ?

Dean Chalk
la source
18
C'est juste mon opinion, mais je pense qu'il y a une règle de plus, qui est très facilement oubliée sous la pile de divers acronymes - "Principe du sens commun". Quand une "solution" crée plus de problèmes qu'elle résout réellement, alors quelque chose ne va pas. Mon point de vue est que si un problème est complexe, mais est enfermé dans une classe qui prend en charge ses complexités et est encore relativement facile à déboguer - je le laisse tranquille. En général, votre idée de «wrapper» me semble juste, mais je vais laisser la réponse à quelqu'un de plus informé.
Patryk Ćwiek
6
En ce qui concerne la «raison de changer», il n’est pas nécessaire de spéculer prématurément sur toutes les raisons. Attendez de devoir changer cela, puis voyez ce qui peut être fait pour faciliter ce genre de changement à l'avenir.
62
Une classe avec 20 paramètres constructeur ne me semble pas très SRP!
MattDavey
1
Vous écrivez "... enregistrement et résolution IoC ..."; cela ressemble à si vous (ou votre chef d'équipe) pensiez que "IoC" et "injection de dépendance" (DI) étaient la même chose, ce qui n'est pas vrai. DI est un moyen de réaliser l'IoC, mais certainement pas le seul. Vous devriez soigneusement analyser pourquoi vous voulez faire IoC; si c'est parce que vous voulez écrire des tests unitaires, vous pouvez également essayer d'utiliser le modèle de localisateur de service ou simplement l'interface classes ( ISomething). IMHO, ces approches sont beaucoup plus faciles à gérer que l'injection de dépendance et donnent un code plus lisible.
2
toute réponse donnée ici serait dans le vide; il faudrait voir le code pour donner une réponse spécifique. 20 paramètres dans un constructeur? eh bien, il vous manque peut-être un objet ... ou ils peuvent tous être valides; ou ils pourraient appartenir dans un fichier de configuration, ou ils peuvent appartenir à une classe DI ou ... Les symptômes sonnent certainement suspect, mais comme la plupart des choses dans CS, « ça dépend » ...
Steven A. Lowe

Réponses:

85

Si votre classe a 20 paramètres dans le constructeur, il ne semble pas que votre équipe sache très bien ce qu'est SRP. Si vous avez une classe qui ne fait qu'une chose, comment a-t-elle 20 dépendances? C'est comme faire une sortie de pêche et apporter une canne à pêche, une boîte à pêche, du matériel de matelassage, une boule de bowling, des nunchucks, un lance-flammes, etc. Si vous avez besoin de tout cela pour pêcher, vous ne vous contentez pas de pêcher.

Cela dit, le PRS, comme la plupart des principes, peut être appliqué de manière excessive. Si vous créez une nouvelle classe pour incrémenter des entiers, alors oui, cela peut être une responsabilité unique, mais bon. C'est ridicule. Nous avons tendance à oublier que des principes tels que les principes SOLID ont un but. SOLID est un moyen de parvenir à une fin, pas une fin en soi. La fin est la maintenabilité . Si vous voulez obtenir cette granularité avec le principe de responsabilité unique, cela signifie que le zèle pour SOLID a aveuglé l'équipe par rapport à l'objectif de SOLID.

Donc, je suppose que ce que je dis, c'est que ... Le PÉR n'est pas votre problème. C'est soit un malentendu sur le PÉR, soit une application incroyablement granulaire. Essayez de faire en sorte que votre équipe garde l'essentiel. Et l’essentiel est la maintenabilité.

MODIFIER

Amenez les gens à concevoir des modules de manière à encourager la facilité d'utilisation. Pensez à chaque classe comme une mini API. Pensez d'abord: «Comment voudrais-je utiliser cette classe», puis l'implémenter. Ne vous contentez pas de penser "Qu'est-ce que cette classe doit faire?" Le PÉR a tendance à rendre les classes plus difficiles à utiliser, si vous n’accordez pas beaucoup d’attention à la convivialité.

EDIT 2

Si vous recherchez des conseils sur la refactorisation, vous pouvez commencer à faire ce que vous avez suggéré: créer des classes plus grossières pour en intégrer plusieurs autres. Assurez-vous que la classe à grains plus grossiers adhère toujours au PÉR , mais à un niveau supérieur. Ensuite, vous avez deux alternatives:

  1. Si les classes à grain fin ne sont plus utilisées ailleurs dans le système, vous pouvez progressivement extraire leur implémentation dans la classe à grain plus grossier et les supprimer.
  2. Laissez les classes à grain fin seules. Peut-être qu'ils ont été bien conçus et que vous aviez simplement besoin de l'emballage pour les rendre plus faciles à utiliser. Je suppose que c'est le cas pour une grande partie de votre projet.

Lorsque vous avez terminé la refactorisation (mais avant de vous engager dans le référentiel), passez en revue votre travail et demandez-vous si votre refactorisation était réellement une amélioration de la maintenabilité et de la facilité d'utilisation.

Phil
la source
2
Autre moyen de faire réfléchir les gens à la conception des classes: laissez-les écrire des cartes CRC (Nom de la classe, Responsabilité, Collaborateurs) . Si une classe a trop de collaborateurs ou de responsabilités, ce n'est probablement pas assez SRP-ish. En d'autres termes, tout le texte doit tenir dans la fiche, sinon il en fait trop.
Spoike
18
Je sais à quoi sert le lance-flammes, mais comment diable pêchez-vous avec une perche?
R. Martinho Fernandes
13
+1 SOLID est un moyen de parvenir à une fin, pas une fin en soi.
B Seven
1
+1: J'ai déjà expliqué que des choses comme "La loi de Demeter" sont mal nommées, cela devrait être "La ligne de repère de Demeter". Ces choses devraient fonctionner pour vous, vous ne devriez pas travailler pour elles.
Binary Worrier
2
@EmmadKareem: Il est vrai que les objets DAO sont supposés avoir plusieurs propriétés. Mais là encore, il y a plusieurs choses que vous pouvez regrouper dans quelque chose d'aussi simple qu'une Customerclasse et qui ont un code plus facile à gérer. Voir des exemples ici: codemonkeyism.com/…
Spoike le
33

Je pense que c'est dans le Refactoring de Martin Fowler que j'ai déjà lu une contre-règle à SRP, définissant où cela va trop loin. Il y a une deuxième question, aussi importante que "chaque classe n'a-t-elle qu'une seule raison de changer?" et c’est-à-dire "chaque changement ne concerne-t-il qu'une classe?"

Si la réponse à la première question est, dans tous les cas, «oui» mais que la deuxième question est «même pas proche», alors vous devez réexaminer la manière dont vous implémentez SRP.

Par exemple, si l'ajout d'un champ à une table signifie que vous devez modifier un DTO, une classe de validation, une classe de persistance et un objet de modèle de vue, etc., vous avez créé un problème. Peut-être devriez-vous repenser la manière dont vous avez implémenté SRP.

Vous avez peut-être dit que l'ajout d'un champ est la raison pour laquelle vous souhaitez modifier l'objet Client, mais le fait de modifier la couche de persistance (par exemple, d'un fichier XML à une base de données) est une autre raison de modifier l'objet Client. Vous décidez donc de créer également un objet CustomerPersistence. Mais si vous le faites de telle sorte que l’ajout d’un champ STILL nécessite une modification de l’objet CustomerPersisitence, alors quel était l’intérêt? Vous avez toujours un objet avec deux raisons de changer - ce n'est tout simplement plus le client.

Cependant, si vous introduisez un ORM, il est tout à fait possible de faire fonctionner les classes de telle sorte que si vous ajoutez un champ au DTO, le SQL utilisé pour lire ces données sera automatiquement modifié. Vous avez alors de bonnes raisons de séparer les deux préoccupations.

En résumé, voici ce que j'ai tendance à faire: s'il y a un équilibre approximatif entre le nombre de fois où je dis "non, il y a plus d'une raison de changer cet objet" et le nombre de fois où je dis "non, ce changement sera affecter plus d'un objet ", alors je pense que j'ai le bon équilibre entre SRP et la fragmentation. Mais si les deux sont toujours élevés, je commence à me demander s'il existe une manière différente de séparer les préoccupations.

pdr
la source
+1 pour "chaque changement n'affecte-t-il qu'une classe?"
dj18
Un problème connexe que je n'ai pas vu discuter est que si les tâches liées à une entité logique sont fragmentées entre différentes classes, il peut être nécessaire que le code contienne des références à plusieurs objets distincts qui sont tous liés à la même entité. Considérons, par exemple, un four avec les fonctions "SetHeaterOutput" et "MeasureTemperature". Si le four était représenté par des objets indépendants HeaterControl et TemperatureSensor, rien n'empêcherait un objet TemperatureFeedbackSystem de contenir une référence à un appareil de chauffage du four et à un capteur de température différent.
Supercat
1
Si, au lieu de cela, ces fonctions étaient combinées dans une interface IKiln, implémentée par un objet Kiln, le TemperatureFeedbackSystem n'aurait besoin que d'une seule référence IKiln. S'il était nécessaire d'utiliser un four avec un capteur de température indépendant, on pourrait utiliser un objet CompositeKiln dont le constructeur acceptait IHeaterControl et ITemperatureSensor et les utilisait pour implémenter IKiln, mais une telle composition volante volontaire serait facilement reconnaissable dans le code.
Supercat
24

Ce n’est pas parce qu’un système est complexe que vous devez le rendre compliqué . Si vous avez une classe qui a trop de dépendances (ou de collaborateurs) comme celle-ci:

public class MyAwesomeClass {
    public class MyAwesomeClass(IDependency1 _d1, IDependency2 _d2, ... , IDependency20 _d20) {
      // Assign it all
    }
}

... alors c'est devenu trop compliqué et vous ne suivez pas vraiment SRP , n'est-ce pas? Je parierais que si vous écrivez ce qui MyAwesomeClassfigure sur une carte CRC, cela ne rentre pas dans une fiche ou vous devez écrire en minuscules lettres illisibles.

Ce que vous avez ici, c'est que vos gars ont seulement suivi le principe de séparation des interfaces et l'ont peut-être poussé à l'extrême, mais c'est une toute autre histoire. Vous pourriez faire valoir que les dépendances sont des objets de domaine (ce qui se produit), mais le fait d’avoir une classe qui gère 20 objets de domaine en même temps l’étire un peu trop.

TDD vous fournira un bon indicateur de ce que fait une classe. Brutalement mis; Si une méthode de test a un code de configuration qui prend du temps à écrire (même si vous refactorisez les tests), vous avez MyAwesomeClassprobablement trop de choses à faire.

Alors, comment résolvez-vous cette énigme? Vous déplacez les responsabilités vers d'autres classes. Vous pouvez prendre certaines mesures pour un cours présentant ce problème:

  1. Identifiez toutes les actions (ou responsabilités) que votre classe fait avec ses dépendances.
  2. Regroupez les actions en fonction de dépendances étroitement liées.
  3. Redéléguer! C'est-à-dire refactoriser chacune des actions identifiées en nouvelles classes ou (plus important encore).

Un exemple abstrait sur les responsabilités de refactoring

Laissez - Cêtre une classe qui a plusieurs dépendances D1, D2, D3, D4que vous devez refactoriser utiliser moins. Lorsque nous identifions les méthodes qui font Cappel aux dépendances, nous pouvons en faire une simple liste:

  • D1- performA(D2),performB()
  • D2 - performD(D1)
  • D3 - performE()
  • D4 - performF(D3)

En regardant la liste, nous pouvons voir cela D1et D2sont liés les uns aux autres car la classe en a besoin d'une manière ou d'une autre. Nous pouvons également voir que les D4besoins D3. Nous avons donc deux groupes:

  • Group 1- D1<->D2
  • Group 2- D4->D3

Les regroupements indiquent que la classe a maintenant deux responsabilités.

  1. Group 1- Un pour gérer l'appelant deux objets qui ont besoin l'un de l'autre. Vous pouvez peut-être laisser votre classe Céliminer le besoin de gérer les deux dépendances et laisser l’une d’elles gérer ces appels à la place. Dans ce groupe, il est évident que l'on D1pourrait faire référence à D2.
  2. Group 2- L’autre responsabilité a besoin d’un objet pour en appeler un autre. Vous ne pouvez pas D4gérer D3au lieu de votre classe? Ensuite, nous pouvons probablement éliminer D3de la classe Cen laissant D4faire les appels à la place.

Ne prenez pas ma réponse comme une pierre dans le marbre, car cet exemple est très abstrait et repose sur de nombreuses hypothèses. Je suis à peu près sûr qu'il y a plus de moyens de reformuler cela, mais au moins, les étapes pourraient vous aider à mettre en place un processus permettant de transférer les responsabilités au lieu de diviser les classes.


Modifier:

Parmi les commentaires, @Emmad Karem dit:

"Si votre classe a 20 paramètres dans le constructeur, il ne semble pas que votre équipe sache ce qu'est SRP. Si vous avez une classe qui ne fait qu'une chose, en quoi a-t-elle 20 dépendances?" - Je pense que avoir une classe client, il n’est pas étrange d’avoir 20 paramètres dans le constructeur.

Il est vrai que les objets DAO ont généralement beaucoup de paramètres, que vous devez définir dans votre constructeur, et que ces paramètres sont généralement de types simples, tels que chaîne. Cependant, dans l'exemple d'une Customerclasse, vous pouvez toujours grouper ses propriétés dans d'autres classes pour simplifier les choses. Par exemple, une Addressclasse avec des rues et une Zipcodeclasse contenant le code postal et gérant la logique métier telle que la validation des données:

public class Address {
    private String street1;
    //...

    private Zipcode zipcode;

    // easy to extend
    public bool isValid() {
        return zipcode.isValid();
    }
}

public class Zipcode {
    private string zipcode;
    public bool isValid() {
        // return regex match that zipcode contains numbers
    }
}

Ce sujet est traité plus en détail dans l'article de blog "Jamais, jamais, n'utilisez jamais String en Java (ou du moins souvent)" . Au lieu d'utiliser des constructeurs ou des méthodes statiques pour faciliter la création des sous-objets, vous pouvez utiliser un modèle de générateur de fluide .

Spoike
la source
+1: bonne réponse! Le regroupement est un mécanisme très puissant à l’OMI car vous pouvez appliquer le regroupement de manière récursive. En gros, avec n couches d’abstraction, vous pouvez organiser 2 ^ n éléments.
Giorgio
+1: Vos premiers paragraphes résument exactement ce à quoi mon équipe est confrontée. Les «objets métier» qui sont en fait des objets de service et le code de configuration du test unitaire qui vous engourdit à écrire. Je savais que nous avions un problème lorsque nos appels de couche service contenaient une ligne de code; un appel à une méthode de couche de gestion.
Homme
3

Je suis d’accord avec toutes les réponses à propos du PÉR et de la manière dont il peut être poussé trop loin. Dans votre message, vous indiquez qu'en raison d'un "sur-refactoring" d'adhésion à SRP, vous avez constaté que l'encapsulation était brisée ou en cours de modification. La seule chose qui a fonctionné pour moi est de toujours rester fidèle à l'essentiel et de faire exactement ce qu'il faut pour arriver à une fin.

Lorsque vous travaillez avec des systèmes Legacy, "l'enthousiasme" de tout réparer pour le rendre meilleur est généralement assez élevé chez les chefs d'équipe, en particulier ceux qui débutent dans ce rôle. SOLID, n’a tout simplement pas de SRP - C’est juste le S. Assurez-vous que si vous suivez SOLID, vous n’oubliez pas non plus le OLID.

Je travaille sur un système Legacy en ce moment et nous avons commencé à suivre un chemin similaire au début. Ce qui a fonctionné pour nous, c’est la décision collective de l’équipe de tirer le meilleur parti des deux mondes - SOLID et KISS (Keep It Simple Stupid). Nous avons collectivement discuté des modifications majeures apportées à la structure du code et appliqué le bon sens dans l’application de divers principes de développement. Ce sont d'excellentes lignes directrices et non des "lois du développement S / W". L’équipe ne se limite pas au chef d’équipe, elle concerne tous les développeurs de l’équipe. Ce qui a toujours fonctionné pour moi, c’est de réunir tout le monde dans une pièce et de proposer un ensemble de directives communes que toute l’équipe accepte de suivre.

En ce qui concerne la résolution de votre situation actuelle, si vous utilisez un VCS et n’ajoutez pas trop de nouvelles fonctionnalités à votre application, vous pouvez toujours revenir à une version du code que l’ensemble de l’équipe considère comme compréhensible, lisible et maintenable. Oui! Je vous demande de jeter du travail et de repartir de zéro. C'est mieux que d'essayer de "réparer" quelque chose qui a été cassé et de le ramener à quelque chose qui existait déjà.

Sharath Satish
la source
3

La réponse est la maintenabilité et la clarté du code avant tout. Pour moi, cela signifie écrire moins de code , pas plus. Moins d'abstractions, moins d'interfaces, moins d'options, moins de paramètres.

Chaque fois que j'évalue une restructuration de code ou que j'ajoute une nouvelle fonctionnalité, je pense à la quantité d'énergie requise par rapport à la logique réelle. Si la réponse est supérieure à 50%, cela signifie probablement que je réfléchis trop.

En plus de SRP, il existe de nombreux autres styles de développement. Dans votre cas, on dirait que YAGNI fait défaut.

cmcginty
la source
3

Beaucoup de réponses ici sont vraiment bonnes mais se concentrent sur le côté technique de cette question. J'ajouterai simplement que cela ressemble aux tentatives du développeur de suivre le SRP comme si elles violaient le SRP.

Vous pouvez voir le blog de Bob ici sur cette situation, mais il affirme que si une responsabilité est répartie entre plusieurs classes, le PRS est violé car ces classes changent en parallèle. Je pense que votre développeur aimerait beaucoup le design en haut du blog de Bob, et pourrait être un peu déçu de le voir déchiré. En particulier parce qu'il enfreint le "principe de clôture commun" - les choses qui changent ensemble restent ensemble.

Rappelez-vous que le PÉR fait référence à la "raison du changement" et non à "faire une chose", et que vous n'avez pas besoin de vous préoccuper de cette raison avant qu'un changement ne se produise réellement. Le deuxième gars paie pour l'abstraction.

Maintenant, il y a le deuxième problème - "l'avocat virulent du développement SOLID". Il ne semble pas que vous ayez une bonne relation avec ce développeur. Par conséquent, toute tentative de le convaincre des problèmes de la base de code est déroutante. Vous aurez besoin de réparer la relation pour pouvoir réellement discuter des problèmes. Ce que je recommanderais, c'est de la bière.

Non sérieusement - si vous ne buvez pas, dirigez-vous vers un café. Sortez du bureau et détendez-vous dans un endroit où vous pourrez parler de ces choses de manière informelle. Plutôt que d'essayer de gagner une dispute lors d'une réunion, ce que vous ne voudrez pas, discutez quelque part de façon amusante. Essayez de reconnaître que ce développeur, qui vous rend dingue, est un humain fonctionnel qui essaie d'obtenir un logiciel "par la porte" et ne veut pas envoyer de la merde. Puisque vous partagez probablement ce terrain d’entente, vous pouvez commencer à discuter de la façon d’améliorer la conception tout en restant conforme au PRS.

Si vous pouvez tous les deux reconnaître que le SRP est une bonne chose, que vous interprétez simplement les aspects différemment, vous pouvez probablement commencer à avoir des conversations productives.

Eric Smith
la source
-1

Je suis d'accord avec votre décision de chef d'équipe [update = 2012.05.31] que le PRS est généralement une bonne idée. Mais je suis tout à fait d'accord avec @ Spoike -s pour dire qu'un constructeur avec 20 arguments d'interface est beaucoup trop. [/ Update]:

L'introduction de SRP avec IoC déplace la complexité d' une "classe multi-responsable" vers plusieurs classes de srp et une initialisation beaucoup plus compliquée au profit de

  • testabilité unitaire / tdd plus facile (tester une classe srp de manière isolée)
  • mais au prix de
    • une initialisation et une intégration de code beaucoup plus difficiles et
    • débogage plus difficile
    • fragmentation (= distribution du code sur plusieurs fichiers / répertoires)

Je crains que vous ne pouvez pas réduire la fragmentation du code sans sacrifier le Srp.

Mais vous pouvez "alléger" le processus d'initialisation du code en implémentant une classe de sucre syntaxique masquant la complexité de l'initialisation dans un constructeur.

   class MySrpClass {
      MySrpClass(Interface1 parm1, Interface2 param2, .... Interface20 param2) {
      }
   } 

   class MySyntaxSugarClass : MySrpClass {
      MySyntaxSugarClass() {
         super(new MyInterface1Implementation(), new MyImpl2(), ....)
      }
   }
k3b
la source
2
Je crois que 20 interfaces est un indicateur que la classe a trop à faire. C'est-à-dire qu'il y a 20 raisons pour que cela change, ce qui est quasiment une violation du PÉR. Ce n'est pas parce que le système est complexe que cela doit être compliqué.
Spoike