Quels sont les moyens pratiques de mettre en œuvre le SRP?

11

Quelles sont simplement les techniques pratiques que les gens utilisent pour vérifier si une classe viole le principe de responsabilité unique?

Je sais qu'une classe ne devrait avoir qu'une seule raison de changer, mais cette phrase manque quelque peu d'un moyen pratique de vraiment mettre cela en œuvre.

La seule façon que j'ai trouvée est d'utiliser la phrase "Le ......... devrait ......... lui-même." où le premier espace est le nom de la classe et le dernier est le nom de la méthode (responsabilité).

Cependant, il est parfois difficile de déterminer si une responsabilité viole vraiment le SRP.

Existe-t-il d'autres moyens de vérifier le SRP?

Remarque:

La question n'est pas de savoir ce que signifie le SRP, mais plutôt une méthodologie pratique ou une série d'étapes pour vérifier et mettre en œuvre le SRP.

MISE À JOUR

Classe de rapport

J'ai ajouté un exemple de classe qui viole clairement le SRP. Ce serait formidable si les gens pouvaient l'utiliser comme exemple pour expliquer comment ils abordent le principe de la responsabilité unique.

L'exemple vient d' ici .

Songo
la source
C'est une règle intéressante, mais vous pouvez toujours écrire: "Une classe de personne peut se rendre". Cela peut être considéré comme une violation pour SRP, car l'inclusion de l'interface graphique dans la même classe qui contient des règles métier et la persistance des données n'est pas OK. Je pense donc que vous devez ajouter le concept de domaines architecturaux (niveaux et couches) et vous assurer que cette déclaration est valide avec 1 de ces domaines uniquement (comme l'interface graphique, l'accès aux données, etc.)
NoChance
@EmmadKareem Cette règle a été mentionnée dans Head First Object-Oriented Analysis and Design et c'est exactement ce que j'en pensais. Il manque quelque peu un moyen pratique de le mettre en œuvre. Ils ont mentionné que parfois les responsabilités ne seront pas aussi évidentes pour le concepteur et qu'il doit faire preuve de beaucoup de bon sens pour juger si la méthode doit vraiment être dans cette classe ou non.
Songo
Si vous voulez vraiment comprendre SRP, lisez quelques-uns des écrits de l'oncle Bob Martin. Son code est parmi les plus jolis que j'ai vus, et je suis convaincu que tout ce qu'il dit à propos de SRP n'est pas seulement un bon conseil, mais aussi plus qu'un simple geste de la main.
Robert Harvey
Et le votant pourrait-il expliquer pourquoi améliorer le poste?!
Songo

Réponses:

7

Le SRP déclare, en termes non équivoques, qu'une classe ne devrait avoir qu'une seule raison de changer.

Déconstruisant la classe "report" dans la question, elle a trois méthodes:

  • printReport
  • getReportData
  • formatReport

En ignorant la redondance Reportutilisée dans chaque méthode, il est facile de voir pourquoi cela viole le SRP:

  • Le terme "impression" implique une sorte d'interface utilisateur ou une imprimante réelle. Cette classe contient donc une certaine quantité d'interface utilisateur ou de logique de présentation. Une modification des exigences de l'interface utilisateur nécessitera une modification de la Reportclasse.

  • Le terme "données" implique une structure de données quelconque, mais ne précise pas vraiment quoi (XML? JSON? CSV?). Quoi qu'il en soit, si le «contenu» du rapport change un jour, cette méthode changera également. Il y a couplage à une base de données ou à un domaine.

  • formatReportest juste un nom terrible pour une méthode en général, mais je suppose en le regardant qu'elle a encore une fois quelque chose à voir avec l'interface utilisateur, et probablement un aspect différent de l'interface utilisateur printReport. Donc, une autre raison sans rapport avec le changement.

Donc, cette seule classe est éventuellement couplée à une base de données, un périphérique écran / imprimante et une logique de formatage interne pour les journaux ou la sortie de fichiers, etc. En regroupant les trois fonctions dans une même classe, vous multipliez le nombre de dépendances et triplez la probabilité que tout changement de dépendance ou d'exigence casse cette classe (ou autre chose qui en dépend).

Une partie du problème ici est que vous avez choisi un exemple particulièrement épineux. Vous ne devriez probablement pas avoir de classe appelée Report, même si cela ne fait qu'une chose , car ... quel rapport? Tous les «rapports» ne sont-ils pas tous des bêtes complètement différentes, basées sur des données différentes et des exigences différentes? Et un rapport n'est-il pas quelque chose qui a déjà été formaté, que ce soit pour l'écran ou pour l'impression?

Mais, en regardant au-delà de cela et en créant un nom concret hypothétique - appelons-le IncomeStatement(un rapport très courant) - une architecture "SRP" appropriée aurait trois types:

  • IncomeStatement- le domaine et / ou la classe de modèle qui contient et / ou calcule les informations qui apparaissent sur les rapports formatés.

  • IncomeStatementPrinter, qui implémenterait probablement une interface standard comme IPrintable<T>. A une méthode clé Print(IncomeStatement), et peut-être d'autres méthodes ou propriétés pour configurer les paramètres spécifiques à l'impression.

  • IncomeStatementRenderer, qui gère le rendu d'écran et est très similaire à la classe d'imprimante.

  • Vous pourriez également éventuellement ajouter des classes plus spécifiques aux fonctionnalités comme IncomeStatementExporter/ IExportable<TReport, TFormat>.

Cela est rendu beaucoup plus facile dans les langages modernes avec l'introduction de génériques et de conteneurs IoC. La plupart de votre code d'application n'a pas besoin de s'appuyer sur la IncomeStatementPrinterclasse spécifique , il peut utiliser IPrintable<T>et donc fonctionner sur tout type de rapport imprimable, ce qui vous donne tous les avantages perçus d'une Reportclasse de base avec une printméthode et aucune des violations SRP habituelles . L'implémentation réelle ne doit être déclarée qu'une seule fois, dans l'enregistrement du conteneur IoC.

Certaines personnes, confrontées à la conception ci-dessus, répondent par quelque chose comme: "mais cela ressemble à du code procédural, et le but de la POO était de nous éloigner - de la séparation des données et du comportement!" À quoi je dis: mal .

Ce IncomeStatementn'est pas seulement des "données", et l'erreur mentionnée ci-dessus est ce qui fait que beaucoup de gens OOP sentent qu'ils font quelque chose de mal en créant une classe aussi "transparente" et par la suite commencent à brouiller toutes sortes de fonctionnalités non liées dans le IncomeStatement(enfin, que et paresse générale). Cette classe peut commencer comme de simples données mais, avec le temps, c'est garanti, elle finira comme un modèle .

Par exemple, un état des revenus réels comprend les revenus totaux , les dépenses totales et les lignes de revenus nets . Un système financier bien conçu ne les stockera probablement pas car il ne s'agit pas de données transactionnelles - en fait, elles changent en fonction de l'ajout de nouvelles données transactionnelles. Cependant, le calcul de ces lignes sera toujours exactement le même, que vous imprimiez, rendiez ou exportiez le rapport. Ainsi , votre IncomeStatementclasse va avoir une quantité juste de comportement sous la forme de getTotalRevenues(), getTotalExpenses()et des getNetIncome()méthodes, et probablement plusieurs autres. C'est un véritable objet de style OOP avec son propre comportement, même s'il ne semble pas vraiment "faire" beaucoup.

Mais les méthodes formatet print, elles n'ont rien à voir avec les informations elles-mêmes. En fait, il n'est pas trop improbable que vous souhaitiez avoir plusieurs implémentations de ces méthodes, par exemple une déclaration détaillée pour la direction et une déclaration moins détaillée pour les actionnaires. La séparation de ces fonctions indépendantes dans différentes classes vous donne la possibilité de choisir différentes implémentations au moment de l'exécution sans la charge d'une print(bool includeDetails, bool includeSubtotals, bool includeTotals, int columnWidth, CompanyLetterhead letterhead, ...)méthode universelle. Beurk!

J'espère que vous pourrez voir où la méthode ci-dessus, massivement paramétrée, va mal, et où les implémentations distinctes vont bien; dans le cas d'un seul objet, chaque fois que vous ajoutez une nouvelle ride à la logique d'impression, vous devez changer votre modèle de domaine ( Tim in finance veut des numéros de page, mais uniquement sur le rapport interne, pouvez-vous ajouter cela? ) par opposition à en ajoutant simplement une propriété de configuration à une ou deux classes satellites à la place.

L'implémentation correcte du SRP consiste à gérer les dépendances . En un mot, si une classe fait déjà quelque chose d'utile et que vous envisagez d'ajouter une autre méthode qui introduirait une nouvelle dépendance (comme une interface utilisateur, une imprimante, un réseau, un fichier, peu importe), ne le faites pas . Pensez à la façon dont vous pourriez ajouter cette fonctionnalité dans une nouvelle classe à la place, et comment vous pourriez faire en sorte que cette nouvelle classe s'intègre dans votre architecture globale (c'est assez facile lorsque vous concevez l'injection de dépendances). C'est le principe / processus général.


Note latérale: Comme Robert, je rejette manifestement l'idée qu'une classe conforme à SRP ne devrait avoir qu'une ou deux variables d'état. On pourrait rarement s'attendre à ce qu'une telle enveloppe mince fasse quelque chose de vraiment utile. Alors n'allez pas trop loin avec ça.

Aaronaught
la source
+1 bonne réponse en effet. Cependant, je suis juste confus au sujet de la classe IncomeStatement. La conception que vous proposez signifie-t-elle que le IncomeStatementaura des instances de IncomeStatementPrinter& de IncomeStatementRenderersorte que lorsque j'appellerai print(), IncomeStatementil déléguera l'appel à la IncomeStatementPrinterplace?
Songo
@Songo: Absolument pas! Vous ne devriez pas avoir de dépendances cycliques si vous suivez SOLID. Apparemment , ma réponse n'a pas fait assez clair que la IncomeStatementclasse n'a pas une printméthode ou une formatméthode, ou toute autre méthode qui ne traite pas directement avec l' inspection ou de manipuler les données du rapport lui - même. C'est à ça que servent ces autres classes. Si vous voulez en imprimer un, vous prenez alors une dépendance à l' IPrintable<IncomeStatement>interface qui est enregistrée dans le conteneur.
Aaronaught
aah je vois votre point. Cependant, où est la dépendance cyclique si j'injecte une Printerinstance dans la IncomeStatementclasse? la façon dont je l'imagine quand je l'appelle IncomeStatement.print()le déléguera IncomeStatementPrinter.print(this, format). Quel est le problème avec cette approche? ... Une autre question, Vous avez mentionné que IncomeStatementdevrait contenir les informations qui apparaissent sur les rapports formatés si je veux qu'elles soient lues à partir de la base de données ou d'un fichier XML, dois-je extraire la méthode qui charge les données dans une classe distincte et lui déléguer l'appel IncomeStatement?
Songo
@Songo: Vous avez IncomeStatementPrinterselon IncomeStatementet IncomeStatementselon IncomeStatementPrinter. C'est une dépendance cyclique. Et c'est juste une mauvaise conception; il n'y a aucune raison IncomeStatementde savoir quoi que ce soit sur un Printerou IncomeStatementPrinter- c'est un modèle de domaine, cela ne concerne pas l'impression, et la délégation est inutile car toute autre classe peut créer ou acquérir un IncomeStatementPrinter. Il n'y a aucune bonne raison d'avoir une notion d'impression dans le modèle de domaine.
Aaronaught
Quant à la façon dont vous chargez le IncomeStatementdepuis la base de données (ou fichier XML) - généralement, cela est géré par un référentiel et / ou un mappeur, pas le domaine, et encore une fois, vous ne déléguez pas cela dans le domaine; si une autre classe a besoin de lire l' un de ces modèles, elle demande explicitement ce référentiel . À moins que vous n'implémentiez le modèle Active Record, je suppose, mais je ne suis vraiment pas un fan.
Aaronaught
2

La façon dont je vérifie le SRP est de vérifier chaque méthode (responsabilité) d'une classe et de poser la question suivante:

"Aurai-je besoin de changer la façon dont j'implémenter cette fonction?"

Si je trouve une fonction que j'aurai besoin d'implémenter de différentes manières (selon une sorte de configuration ou de condition), alors je sais avec certitude que j'ai besoin d'une classe supplémentaire pour gérer cette responsabilité.

John Raya
la source
1

Voici une citation de la règle 8 de la callisthénie des objets :

La plupart des classes devraient simplement être responsables de la gestion d'une seule variable d'état, mais il en existe quelques-unes qui en nécessitent deux. L'ajout d'une nouvelle variable d'instance à une classe diminue immédiatement la cohésion de cette classe. En général, lors de la programmation selon ces règles, vous constaterez qu'il existe deux types de classes, celles qui conservent l'état d'une variable d'instance unique et celles qui coordonnent deux variables distinctes. En général, ne mélangez pas les deux types de responsabilités

Compte tenu de cette vue (quelque peu idéaliste), on pourrait dire que toute classe qui ne contient qu'une ou deux variables d'état ne risque pas de violer SRP. Vous pouvez également dire que toute classe qui contient plus de deux variables d'état peut violer SRP.

MattDavey
la source
2
Cette vue est désespérément simpliste. Même l'équation célèbre mais simple d'Einstein nécessite deux variables.
Robert Harvey
La question des PO était "Y a-t-il d'autres façons de vérifier le SRP?" - c'est un indicateur possible. Oui, c'est simpliste et cela ne tient pas dans tous les cas, mais c'est un moyen possible de vérifier que SRP a été violé.
MattDavey
1
Je soupçonne que l'état mutable vs immuable est également une considération importante
jk.
La règle 8 décrit le processus parfait pour créer des conceptions qui ont des milliers et des milliers de classes, ce qui rend le système désespérément complexe, incompréhensible et incontrôlable. Mais le côté positif est que vous pouvez suivre SRP.
Dunk
@Dunk, je ne suis pas en désaccord avec vous, mais cette discussion est entièrement hors sujet pour la question.
MattDavey
1

Une implémentation possible (en Java). J'ai pris des libertés avec les types de retour mais dans l'ensemble je pense que cela répond à la question. TBH Je ne pense pas que l'interface avec la classe Report soit si mauvaise, bien qu'un meilleur nom puisse être en ordre. J'ai laissé de côté les déclarations et les assertions de la garde par souci de concision.

EDIT: Notez également que la classe est immuable. Donc, une fois créé, vous ne pouvez rien changer. Vous pouvez ajouter un setFormatter () et un setPrinter () et ne pas avoir trop de problèmes. La clé, à mon humble avis, est de ne pas modifier les données brutes après l'instanciation.

public class Report
{
    private ReportData data;
    private ReportDataDao dao;
    private ReportFormatter formatter;
    private ReportPrinter printer;


    /*
     *  Parameterized constructor for depndency injection, 
     *  there are better ways but this is explicit.
     */
    public Report(ReportDataDao dao, 
        ReportFormatter formatter, ReportPrinter printer)
    {
        super();
        this.dao = dao;
        this.formatter = formatter;
        this.printer = printer;
    }

    /*
     * Delegates to the injected printer.
     */
    public void printReport()
    {
        printer.print(formatReport());
    }


    /*
     * Lazy loading of data, delegates to the dao 
     * for the meat of the call.
     */
    public ReportData getReportData()
    {
        if (reportData == null)
        {
            reportData = dao.loadData();
        }
        return reportData;
    }

    /*
     * Delegate to the formatter for formatting 
     * (notice a pattern here).
     */
    public ReportData formatReport()
    {
        formatter.format(getReportData());
    }
}
Heath Lilley
la source
Merci pour l'implémentation. J'ai 2 choses, dans la ligne, if (reportData == null)je suppose que vous voulez dire à la dataplace. Deuxièmement, j'espérais savoir comment vous en êtes arrivé à cette mise en œuvre. Comme pourquoi avez-vous décidé de déléguer tous les appels à d'autres objets à la place. Encore une chose sur laquelle je me suis toujours posé la question: est-ce vraiment la responsabilité d'un rapport de s'imprimer?! Pourquoi n'avez-vous pas créé une printerclasse distincte qui prend un reportdans son constructeur?
Songo
Oui, reportData = data, désolé. La délégation permet un contrôle précis des dépendances. Au moment de l'exécution, vous pouvez fournir des implémentations alternatives pour chaque composant. Vous pouvez maintenant avoir un HtmlPrinter, PdfPrinter, JsonPrinter, ... etc. Ceci est également pratique pour les tests car vous pouvez tester vos composants délégués de manière isolée ainsi qu'intégrés dans l'objet ci-dessus. Vous pouvez certainement inverser la relation entre l'imprimante et le rapport, je voulais juste montrer qu'il était possible de fournir une solution avec l'interface de classe fournie. C'est une habitude de travailler sur des systèmes hérités. :)
Heath Lilley
hmmmm ... Donc, si vous construisiez le système à partir de zéro, quelle option prendriez-vous? Une Printerclasse qui prend un rapport ou une Reportclasse qui prend une imprimante? J'ai rencontré un problème similaire avant où je devais analyser un rapport et je me suis disputé avec mon TL si nous devions créer un analyseur qui prend un rapport ou si le rapport devrait avoir un analyseur à l'intérieur et l' parse()appel lui est délégué.
Songo
Je ferais les deux ... printer.print (report) pour démarrer et report.print () si nécessaire plus tard. La grande chose au sujet de l'approche printer.print (rapport) est qu'elle est hautement réutilisable. Il sépare la responsabilité et vous permet d'avoir des méthodes pratiques là où vous en avez besoin. Peut-être que vous ne voulez pas que d'autres objets de votre système soient informés de ReportPrinter, donc en ayant une méthode print () sur une classe, vous atteignez un niveau d'abstrait qui isole votre logique d'impression de rapport du monde extérieur. Cela a toujours un vecteur de changement étroit et est facile à utiliser.
Heath Lilley
0

Dans votre exemple, il n'est pas clair que le SRP est violé. Peut-être que le rapport devrait pouvoir se mettre en forme et s’imprimer, s’ils sont relativement simples:

class Report {
  void format() {
     text = text.trim();
  }

  void print() {
     new Printer().write(text);
  }
}

Les méthodes sont si simples , il n'a pas de sens d'avoir ReportFormatterou ReportPrinterclasses. Le seul problème flagrant dans l'interface est getReportDataparce qu'il viole ask don't tell sur un objet sans valeur.

D'un autre côté, si les méthodes sont très compliquées ou qu'il existe de nombreuses façons de formater ou d'imprimer un, Reportil est logique de déléguer la responsabilité (également plus testable):

class Report {
  void format(ReportFormatter formatter) {
     text = formatter.format(text);
  }

  void print(ReportPrinter printer) {
     printer.write(text);
  }
}

SRP est un principe de conception et non un concept philosophique et il est donc basé sur le code réel avec lequel vous travaillez. Sémantiquement, vous pouvez diviser ou regrouper une classe en autant de responsabilités que vous le souhaitez. Cependant, en tant que principe pratique, SRP devrait vous aider à trouver le code que vous devez modifier . Les signes que vous violez SRP sont:

  • Les classes sont si grandes que vous perdez du temps à faire défiler ou à chercher la bonne méthode.
  • Les classes sont si petites et nombreuses que vous perdez du temps à sauter entre elles ou à trouver la bonne.
  • Quand vous devez faire un changement, cela affecte tellement de classes qu'il est difficile de garder une trace.
  • Lorsque vous devez effectuer un changement, il est difficile de savoir ce que les classes doivent changer.

Vous pouvez résoudre ces problèmes en refactorisant en améliorant les noms, en regroupant du code similaire, en éliminant la duplication, en utilisant une conception en couches et en divisant / combinant les classes selon les besoins. La meilleure façon d'apprendre la SRP est de plonger dans une base de code et de refactoriser la douleur.

Garrett Hall
la source
Pourriez-vous vérifier l’exemple que j’ai joint au message et élaborer votre réponse en fonction de celui-ci.
Songo
Mise à jour. SRP dépend du contexte, si vous avez posté une classe entière (dans une question séparée), il serait plus facile de l'expliquer.
Garrett Hall
Merci pour la mise à jour. Une question cependant, est-ce vraiment la responsabilité d'un rapport de s'imprimer?! Pourquoi n'avez-vous pas créé une classe d'imprimante distincte qui prend un rapport dans son constructeur?
Songo
Je dis simplement que SRP dépend du code lui-même, vous ne devriez pas l'appliquer de manière dogmatique.
Garrett Hall
oui je comprends votre point. Mais si vous construisiez le système à partir de zéro, quelle option choisiriez-vous? Une Printerclasse qui prend un rapport ou une Reportclasse qui prend une imprimante? Plusieurs fois, je suis confronté à une telle question de conception avant de déterminer si le code se révélera complexe ou non.
Songo
0

Le principe de responsabilité unique est étroitement lié à la notion de cohésion . Afin d'avoir une classe hautement cohésive, vous devez avoir une co-dépendance entre les variables d'instance de la classe et ses méthodes; c'est-à-dire que chacune des méthodes doit manipuler autant de variables d'instance que possible. Plus une méthode utilise de variables, plus sa classe est cohérente; une cohésion maximale est généralement impossible.

De plus, pour bien appliquer la SRP, vous comprenez bien le domaine de la logique métier; pour savoir ce que chaque abstraction doit faire. L'architecture en couches est également liée à SRP, en faisant en sorte que chaque couche fasse une chose spécifique (la couche source de données doit fournir des données, etc.).

Pour en revenir à la cohésion même si vos méthodes n'utilisent pas toutes les variables, elles doivent être couplées:

public class MyClass {
    private Type1 var1;
    private Type2 var2;
    private Type3 var3;

    public Type3 method1() {
        //use var1 and var3
    }  

    public void method2() {
        //use var1 and var2
    }

    public Type1 method3() {
        //use var2 and var3
    }
}

Vous ne devriez pas avoir quelque chose comme le code ci-dessous, où une partie des variables d'instance sont utilisées dans une partie des méthodes, et l'autre partie des variables sont utilisées dans l'autre partie des méthodes (ici, vous devriez avoir deux classes pour chaque partie des variables).

public class MyClass {
    private Type1 var1;
    private Type2 var2;
    private Type3 var3;
    private TypeA varA;
    private TypeB varB;

    public Type3 method1() {
        //use var1 and var3
    }  

    public void method2() {
        //use var1 and var2
    }

    public TypeA methodA() {
        //use varA and varB
    }

    public TypeA methodB() {
        //use varA
    }
}
m3th0dman
la source