Principe de responsabilité unique - suis-je en train d'en abuser?

13

Pour référence - http://en.wikipedia.org/wiki/Single_responsibility_principle

J'ai un scénario de test dans lequel dans un module d'application est responsable de la création des entrées du grand livre. Il y a trois tâches de base qui pourraient être effectuées -

  • Affichez les écritures existantes au format tableau.
  • Créez une nouvelle entrée de grand livre à l'aide du bouton Créer.
  • Cliquez sur une entrée du grand livre dans le tableau (mentionnée au premier pointeur) et affichez ses détails à la page suivante. Vous pouvez annuler une entrée de grand livre sur cette page.

(Il y a quelques opérations / validations supplémentaires sur chaque page, mais par souci de concision, je les limiterai à celles-ci)

J'ai donc décidé de créer trois classes différentes -

  • LedgerLandingPage
  • CreateNewLedgerEntryPage
  • ViewLedgerEntryPage

Ces classes offrent les services qui pourraient être effectués dans ces pages et les tests Selenium utilisent ces classes pour amener l'application à un état où je pourrais faire certaines affirmations.

Quand je l'avais révisé avec l'un de mes collègues, il était submergé et m'a demandé de faire un seul cours pour tous. Bien que je pense que ma conception est très propre, je doute que j'utilise le principe de la responsabilité unique

Tarun
la source
2
À mon humble avis, il est inutile d'essayer de répondre à votre question en général, sans savoir deux choses: Quelle est la taille de ces classes (par nombre de méthodes et LOC)? Et combien devraient-ils grandir / devenir plus complexes dans un avenir prévisible?
Péter Török
2
10 méthodes chacune est à mon humble avis une indication claire de ne pas mettre le code dans une classe avec 30 méthodes.
Doc Brown
6
C'est un territoire limite: une classe de 100 LOC et 10 méthodes n'est pas trop petite, et une des 300 LOC n'est pas trop grande. Cependant, 30 méthodes dans une seule classe me semblent trop. Dans l'ensemble, j'ai tendance à être d'accord avec @Doc en ce sens qu'il y a moins de risques d'avoir de nombreuses petites classes que d'avoir une seule classe en surpoids. Surtout en tenant compte du fait que les cours ont naturellement tendance à prendre du poids au fil du temps ...
Péter Török
1
@ PéterTörök - Je suis d'accord à moins que le code puisse être refactorisé en une classe qui peut être utilisée intuitivement qui réutilise le code au lieu de dupliquer des fonctionnalités comme je m'attends à ce que l'OP en ait.
SoylentGray
1
Peut-être que l'application de MVC clarifierait tout cela: trois vues, un modèle (Ledger) et probablement trois contrôleurs.
Kevin Cline

Réponses:

19

Citant @YannisRizos ici :

C'est une approche instinctive et c'est probablement correct, car ce qui est le plus susceptible de changer, c'est la logique métier de la gestion des grands livres. Si cela se produit, il serait beaucoup plus facile de maintenir une classe que trois.

Je suis en désaccord, au moins maintenant, après environ 30 ans d'expérience en programmation. Les classes plus petites à mon humble avis sont mieux à maintenir dans presque tous les cas, je peux penser à des cas comme celui-ci. Plus vous mettez de fonctions dans une classe, moins vous aurez de structure et la qualité de votre code se dégrade un peu chaque jour. Quand j'ai bien compris Tarun, chacune de ces 3 opérations

LedgerLandingPage

CreateNewLedgerEntryPage

ViewLedgerEntryPage

est un cas d'utilisation, chacune de ces classes se compose de plusieurs fonctions pour effectuer la tâche connexe, et chacune peut être développée et testée séparément. Ainsi, la création de classes pour chacun d'eux regroupe des choses qui appartiennent ensemble, ce qui facilite beaucoup l'identification de la partie du code à modifier si quelque chose doit changer.

Si vous avez des fonctionnalités communes entre ces 3 classes, elles appartiennent soit à une classe de base commune, la Ledgerclasse elle-même (je suppose que vous en avez une, au moins, pour les opérations CRUD), soit à une classe d'assistance distincte.

Si vous avez besoin de plus d'arguments pour créer de nombreuses petites classes, je vous recommande de jeter un œil au livre de Robert Martin "Clean Code" .

Doc Brown
la source
c'est la raison pour laquelle j'ai trouvé trois classes de diff au lieu de tout pousser dans une seule classe. Puisqu'il n'y a aucune fonctionnalité commune parmi ces derniers, je n'ai pas de classe commune. Merci pour le lien.
Tarun
2
"Il est préférable de maintenir des classes plus petites dans presque tous les cas auxquels je pense." Je ne pense pas que même Clean Code irait aussi loin. Diriez-vous vraiment que, dans un modèle MVC, il devrait y avoir un contrôleur par page (ou cas d'utilisation, comme vous le dites), par exemple? Dans Refactoring, Fowler a deux odeurs de code ensemble: l'une se référant à avoir de nombreuses raisons de changer une classe (SRP), et l'autre se référant à devoir éditer plusieurs classes pour une seule modification.
pdr
@pdr, l'OP indique qu'il n'y a pas de fonctionnalité commune entre ces classes, donc (pour moi) il est difficile d'imaginer une situation où vous devez en toucher plusieurs pour effectuer un seul changement.
Péter Török
@pdr: Si vous avez trop de classes à éditer pour un seul changement, c'est surtout un signe que vous n'avez pas refactorisé les fonctionnalités communes vers un endroit séparé. Mais dans l'exemple ci-dessus, cela conduirait probablement à une quatrième classe, et pas seulement une.
Doc Brown
2
@pdr: BTW, avez-vous réellement lu "Clean Code"? Bob Martin va très loin en décomposant le code en unités plus petites.
Doc Brown
11

Il n'y a pas de mise en œuvre définitive du principe de responsabilité unique ou de tout autre principe. Vous devez décider en fonction de ce qui est le plus susceptible de changer.

Comme l'écrit @Karpie dans une réponse précédente :

Vous n'avez besoin que d'une seule classe, et la seule responsabilité de cette classe est de gérer les grands livres.

C'est une approche instinctive et c'est probablement correct, car ce qui est le plus susceptible de changer, c'est la logique métier de la gestion des grands livres. Si cela se produit, il serait beaucoup plus facile de maintenir une classe que trois.

Mais cela devrait être examiné et décidé par cas. Vous ne devriez pas vraiment vous soucier des préoccupations avec une minuscule possibilité de changement et concentrer l'application du principe sur ce qui est le plus susceptible de changer. Si la logique de gestion des registres est un processus multicouche et que les couches sont susceptibles de changer indépendamment ou sont des préoccupations qui devraient être séparées en principe (logique et présentation), vous devez utiliser des classes distinctes. C'est un jeu de probabilité.

Une question similaire au sujet de l'utilisation excessive des principes de conception, que je pense que vous trouveriez intéressante: Modèles de conception: quand utiliser et quand arrêter de tout faire en utilisant des modèles

yannis
la source
1
SRP, à ma connaissance, n'est pas vraiment un modèle de conception.
Doc Brown
@DocBrown Bien sûr, ce n'est pas un modèle de conception, mais la discussion sur la question est extrêmement pertinente ... Bonne capture cependant, j'ai mis à jour la réponse
yannis
4

Examinons ces classes:

  • LedgerLandingPage: Le rôle de cette classe est d'afficher une liste de livres. À mesure que l'application se développe, vous souhaiterez probablement ajouter des méthodes de tri, de filtrage, de surlignage et de fusion de manière transparente dans les données provenant d'autres sources de données.

  • ViewLedgerEntryPage: Cette page montre le grand livre en détail. Semble assez simple. Tant que les données sont simples. Vous pouvez annuler le grand livre ici. Vous pouvez également vouloir le corriger. Ou commentez-le, ou joignez un fichier, un reçu ou un numéro de réservation externe ou autre. Et une fois que vous autorisez l'édition, vous voulez vraiment montrer une histoire.

  • CreateLedgerEntryPage: si le ViewLedgerEntryPagea la pleine capacité d'édition, la responsabilité de cette classe peut éventuellement être remplie en éditant une "entrée vide", qui peut ou non avoir un sens.

Ces exemples peuvent sembler un peu exagérés, mais le fait est que, à partir d'un UX, nous parlons de fonctionnalités ici. Une caractéristique unique est certainement une responsabilité distincte et unique. Parce que les exigences de cette fonctionnalité peuvent changer et que les exigences de deux fonctionnalités différentes peuvent se transformer en deux directions opposées, et que vous souhaitez que vous vous en teniez au SRP depuis le début.
Mais à l'inverse, vous pouvez toujours gifler une façade sur trois classes, si avoir une seule interface est plus pratique pour une raison à laquelle vous pouvez penser.


Il y a une telle chose comme la surutilisation de SRP : si vous avez besoin d'une douzaine de classes pour lire le contenu d'un fichier, il est plutôt sûr de supposer que vous faites exactement cela.

Si vous regardez le PÉR de l'autre côté, il est en fait dit qu'une seule responsabilité devrait être assumée par une seule classe. Veuillez noter cependant que les responsabilités n'existent qu'au sein des niveaux d'abstraction. Il est donc parfaitement logique pour une classe d'un niveau d'abstraction supérieur de s'acquitter de sa responsabilité en déléguant le travail à un composant de niveau inférieur, ce qui est mieux réalisé par l' inversion des dépendances .

back2dos
la source
Je suppose que même je n'aurais pas pu décrire la responsabilité de ces classes mieux que vous.
Tarun
2

Ces classes ont-elles une seule raison de changer?

Si vous ne le savez pas (encore), alors vous pourriez être entré dans le piège de la conception spéculative / de l'ingénierie (trop de classes, des modèles de conception trop complexes appliqués). Vous n'avez pas satisfait YAGNI . Aux premiers stades du cycle de vie du logiciel, certaines exigences peuvent ne pas être claires. Ainsi, la direction du changement n'est actuellement pas visible pour vous. Si vous vous rendez compte de cela, conservez-le dans une ou une quantité minimale de cours. Cependant, cela s'accompagne d'une responsabilité: dès que les exigences et la direction du changement sont plus claires, vous devez repenser la conception actuelle et effectuer une refactorisation pour satisfaire la raison du changement.

Si vous savez déjà qu'il y a trois raisons différentes de changer et qu'elles correspondent à ces trois classes, alors vous venez d'appliquer la bonne dose de SRP. Encore une fois, cela comporte une certaine responsabilité: si vous vous trompez ou que les exigences futures changent de manière inattendue, vous devez repenser la conception actuelle et effectuer une refactorisation pour satisfaire la raison du changement.

Le tout est:

  • Gardez un œil sur les moteurs du changement.
  • Choisissez un design flexible, qui peut être refactorisé.
  • Soyez prêt à refactoriser le code.

Si vous avez cet état d'esprit, vous façonnerez le code sans trop craindre la conception spéculative / l'ingénierie excessive.

Cependant, il y a toujours le facteur temps: pour de nombreux changements, vous n'aurez pas le temps dont vous avez besoin. La refactorisation coûte du temps et des efforts. J'ai souvent rencontré des situations où je savais que je devais changer la conception, mais en raison de contraintes de temps, j'ai dû la reporter. Cela se produira et ne peut être évité. J'ai trouvé qu'il est plus facile de refactoriser sous le code d'ingénierie que sur le code d'ingénierie. YAGNI me donne une bonne ligne directrice: en cas de doute, réduisez au minimum le nombre de cours et l'application de modèles de conception.

Theo Lenndorff
la source
1

Il y a trois raisons d'invoquer SRP pour diviser une classe:

  • de sorte que les changements futurs dans les exigences peuvent laisser certaines classes inchangées
  • afin qu'un bug puisse être isolé plus rapidement
  • pour rendre la classe plus petite

Il y a trois raisons de refuser de diviser une classe:

  • de sorte que les modifications futures des exigences ne vous obligeront pas à effectuer le même changement dans plusieurs classes
  • afin que la recherche de bogues n'implique pas de tracer de longs chemins d'indirection
  • pour résister aux techniques de rupture d'encapsulation (propriétés, amis, etc.) qui exposent les informations ou les méthodes à tout le monde quand elles ne sont nécessaires qu'à une seule classe apparentée

Lorsque j'examine votre cas et que j'imagine des changements, certains d'entre eux entraîneront des changements dans plusieurs classes (par exemple, ajouter un champ au grand livre, vous devrez changer les trois), mais beaucoup ne le feront pas - par exemple, ajouter le tri à la liste des grands livres ou modifier les règles de validation lors de l'ajout d'une nouvelle entrée de grand livre. La réaction de votre collègue est probablement parce qu'il est difficile de savoir où chercher un bug. Si quelque chose ne va pas dans la liste des grands livres, est-ce parce qu'il a été ajouté à tort ou qu'il y a quelque chose de mal dans la liste? S'il y a un écart entre le résumé et les détails, quel est le problème?

Il est également possible que votre collègue pense que les changements dans les exigences qui signifient que vous devrez changer les trois classes sont beaucoup plus probables que ces changements où vous vous en sortiriez en n'en changeant qu'une. Cela pourrait être une conversation vraiment utile.

Kate Gregory
la source
bon de voir une perspective différente
Tarun
0

Je pense que si le grand livre était une table en db, je suggère de mettre ces tâches de thress à une classe (par exemple DAO). Si le grand livre a plus de logique et n'était pas une table en db, je suggère que nous devrions créer plus de classe pour faire ces tâches (peut être deux ou classe thress) et fournir une classe de façade à faire simplement pour le client.

Mark xie
la source
eh bien, le grand livre est une fonctionnalité de l'interface utilisateur et il existe de nombreuses opérations connexes qui pourraient être effectuées à partir de différentes pages.
Tarun
0

À mon humble avis, vous ne devriez pas utiliser du tout les classes. Il n'y a aucune abstraction de données ici. Les registres sont un type de données concret. Les méthodes pour les manipuler et les afficher sont des fonctions et des procédures. Il y aura certainement de nombreuses fonctions couramment utilisées à partager, telles que le formatage d'une date. Il y a peu de place pour que des données soient abstraites et cachées dans une classe dans les routines d'affichage et de sélection.

Il existe certains cas pour masquer l'état dans une classe pour l'édition du grand livre.

Que vous abusiez du SRP ou non, vous abusez de quelque chose de plus fondamental: vous abusez de l'abstraction. C'est un problème typique, engendré par la notion mythique OO est un paradigme de développement sain.

La programmation est à 90% concrète: l'utilisation de l'abstraction des données doit être rare et soigneusement conçue. L'abstraction fonctionnelle, d'autre part, devrait être plus courante, car les détails du langage et le choix des algorithmes doivent être séparés de la sémantique de l'opération.

Yttrill
la source
-1

Vous n'avez besoin que d'une seule classe, et la seule responsabilité de cette classe est de gérer les grands livres .

sevenseacat
la source
2
Pour moi, une classe «... manager» montre généralement que vous ne pourriez pas être dérangé de la décomposer davantage. Qu'est-ce que la classe gère? La présentation, la persistance?
sebastiangeiger
-1. Mettre 3 cas d'utilisation ou plus dans 1 classe est exactement ce que le SRP essaie d'éviter - et pour de bonnes raisons.
Doc Brown
@sebastiangeiger Alors, que font les trois classes du PO? Présentation ou persistance?
sevenseacat
@Karpie J'espère sincèrement une présentation. Certes, l'exemple n'est pas vraiment bon, mais je m'attendrais à ce que «... Page» dans un site Web fasse quelque chose de similaire à une vue.
sebastiangeiger
2
En fait, vous n'avez besoin que d'une seule classe pour toute l'application, avec la seule responsabilité de rendre les utilisateurs heureux ;-)
Péter Török