Cette conception de classe viole-t-elle le principe de responsabilité unique?

Aujourd'hui, je me suis disputé avec quelqu'un.

J'expliquais les avantages d'avoir un modèle de domaine riche par opposition à un modèle de domaine anémique. Et j'ai démontré mon point avec un cours simple ressemblant à ça:

public class Employee
{
    public Employee(string firstName, string lastName)
    {
        FirstName = firstName;
        LastName = lastname;
    }

    public string FirstName { get private set; }
    public string LastName { get; private set;}
    public int CountPaidDaysOffGranted { get; private set;}

    public void AddPaidDaysOffGranted(int numberOfdays)
    {
        // Do stuff
    }
}

Lorsqu'il a défendu son approche de modèle anémique, l'un de ses arguments était le suivant: "Je suis un partisan de SOLID . Vous violez le principe de responsabilité unique (SRP), car vous représentez à la fois des données et une logique dans la même classe."

J'ai trouvé cette affirmation vraiment surprenante, car, à la suite de ce raisonnement, toute classe ayant une propriété et une méthode contrevient au PRS. Par conséquent, la programmation orientée objet n'est généralement pas SOLIDE et la programmation fonctionnelle est le seul moyen de parvenir au paradis.

J'ai décidé de ne pas répondre à ses nombreux arguments, mais je suis curieux de savoir ce que la communauté pense de cette question.

Si j'avais répondu, j'aurais commencé par indiquer le paradoxe mentionné ci-dessus, puis indiquer que le PRS dépend fortement du niveau de granularité que vous souhaitez prendre en compte et que, si vous le prenez suffisamment loin, toute classe contenant plus d'un propriété ou une méthode le viole.

Qu'aurais-tu dit?

Mise à jour: L’exemple a été généreusement mis à jour par Guntbert pour rendre la méthode plus réaliste et nous aider à nous concentrer sur la discussion sous-jacente.

La responsabilité unique doit être comprise comme une abstraction des tâches logiques de votre système. Une classe devrait avoir la responsabilité unique de (faire tout ce qui est nécessaire pour) d’exécuter une tâche unique et spécifique. Cela peut effectivement apporter beaucoup dans une classe bien conçue, en fonction de la responsabilité. La classe qui exécute votre moteur de script, par exemple, peut avoir beaucoup de méthodes et de données impliquées dans le traitement des scripts.

Votre collègue se concentre sur la mauvaise chose. La question n'est pas "quels membres cette classe a-t-elle?" mais "quelle (s) opération (s) utile (s) cette classe exécute-t-elle dans le programme?" Une fois que cela est compris, votre modèle de domaine a l’air parfait.

Le principe de responsabilité unique ne concerne que la question de savoir si un élément de code (en langage OOP, nous parlons généralement de classes) a la responsabilité d'une fonctionnalité . Je pense que votre ami affirmant que les fonctions et les données ne peuvent pas se mélanger n'a pas vraiment compris cette idée. S'il Employeedevait également contenir des informations sur son lieu de travail, sur la vitesse de sa voiture et sur le type de nourriture mangée par son chien, nous aurions un problème.

Étant donné que cette classe ne traite que d'un Employee, je pense qu'il est juste de dire qu'il ne viole pas le SRP de manière flagrante, mais les gens vont toujours avoir leur propre opinion.

Nous pourrions nous améliorer en séparant les informations relatives aux employés (nom, numéro de téléphone, adresse électronique, par exemple) de leurs vacances. Dans mon esprit, cela ne signifie pas que les méthodes et les données ne peuvent pas se mélanger , cela signifie simplement que la fonctionnalité de vacances pourrait peut-être se trouver dans un endroit séparé.

À mon avis, cette classe pourrait potentiellement violer SRP si elle continuait à représenter à la fois un Employeeet EmployeeHolidays.

Dans l’état actuel des choses, et si j’avais l’intention de procéder à un examen par les pairs, je le laisserais probablement passer. Si davantage de propriétés et de méthodes spécifiques aux employés étaient ajoutées, et si plus de propriétés spécifiques à des vacances étaient ajoutées, je recommanderais probablement une scission, citant à la fois le SRP et le FAI.

Il existe déjà d'excellentes réponses soulignant que SRP est un concept abstrait de la fonctionnalité logique, mais il y a des points plus subtils que je pense méritent d'être ajoutés.

Les deux premières lettres de SOLID, SRP et OCP, concernent toutes les deux la modification de votre code en réponse à une modification des exigences. Ma définition préférée du SRP est: "un module / classe / fonction ne devrait avoir qu'une seule raison de changer." Argumenter sur les raisons probables de la modification de votre code est beaucoup plus productif que sur le fait de savoir si votre code est SOLID ou non.

Combien de raisons votre classe d'employés doit-elle changer? Je ne sais pas, car je ne connais pas le contexte dans lequel vous l'utilisez, et je ne vois pas non plus l'avenir. Ce que je peux faire, c'est réfléchir à des changements possibles sur la base de ce que j'ai vu dans le passé, et vous pouvez évaluer subjectivement leur probabilité. Si plus d'un score se situe entre "raisonnablement probable" et "mon code a déjà changé pour cette raison exacte", vous violez SRP contre ce type de modification. En voici un: une personne portant plus de deux noms rejoint votre entreprise (ou un architecte lit cet excellent article du W3C ). En voici un autre: votre entreprise change la manière dont elle attribue les jours de vacances.

Notez que ces raisons sont également valables même si vous supprimez la méthode AddHolidays. De nombreux modèles de domaine anémique violent le PRS. Nombre d'entre elles ne sont que des tables de base de données en code, et il est très courant que les tables de base de données aient plus de 20 raisons de changer.

Voici quelque chose à mâcher: votre classe d'employés changerait-elle si votre système devait suivre les salaires des employés? Des adresses? Informations de contact d'urgence? Si vous disiez «oui» (et «susceptible de se produire») à deux de ces personnes, votre classe violerait alors SRP même si aucun code n'y figurait encore! SOLID concerne les processus et l'architecture autant que le code.

Le fait que la classe représente des données n’est pas la responsabilité de la classe, c’est un détail d’implémentation privé.

La classe a une responsabilité, représenter un employé. Dans ce contexte, cela signifie qu’elle présente une API publique qui vous fournit les fonctionnalités dont vous avez besoin pour traiter avec les employés (le fait de savoir si AddHolidays est un bon exemple est discutable).

La mise en œuvre est interne; il se trouve que cela nécessite des variables privées et une certaine logique. Cela ne signifie pas que la classe a maintenant plusieurs responsabilités.

L'idée selon laquelle mélanger la logique et les données est toujours fausse, est tellement ridicule qu'elle ne mérite même pas d'être discutée. Cependant, l'exemple cite clairement une violation de la responsabilité unique, mais ce n'est pas parce qu'il y a une propriété DaysOfHolidayset une fonction AddHolidays(int).

C'est parce que l'identité de l'employé est mêlée à la gestion des vacances, ce qui est une mauvaise chose. L'identité de l'employé est une chose complexe qui nécessite de suivre les vacances, le salaire, les heures supplémentaires, d'indiquer qui fait partie de quelle équipe, de créer un lien avec les rapports de performance, etc. employé. Les employés peuvent même avoir plusieurs orthographes de leur nom, par exemple un orthographe ASCII et unicode. Les personnes peuvent avoir 0 à n prénom et / ou nom de famille. Ils peuvent avoir des noms différents dans différentes juridictions. Le suivi de l'identité d'un employé est une responsabilité suffisante pour que la gestion des vacances ou des vacances ne puisse être ajoutée sans que cela soit appelé une deuxième responsabilité.

"Je crois fermement à SOLID. Vous violez le principe de responsabilité unique (SRP), car vous représentez des données et exécutez une logique dans la même classe."

Comme les autres, je suis en désaccord avec cela.

Je dirais que le SRP est violé si vous effectuez plus d'une logique dans la classe. La quantité de données à stocker dans la classe pour réaliser cet élément logique n’est pas pertinente.

De nos jours, je ne trouve pas utile de débattre de ce qui constitue ou non une responsabilité ou une raison de changer. Je proposerais un principe de peine minimum à la place:

Principe de deuil minimum: le code doit chercher à minimiser la probabilité d’exiger des modifications ou à en simplifier la modification.

Comment ça Ne devrait pas demander à un spécialiste en sciences de la fusée de comprendre pourquoi cela peut aider à réduire les coûts de maintenance. Espérons que cela ne devrait pas être un point de débat sans fin, mais comme dans le cas de SOLID en général, il n’est pas possible de l’appliquer aveuglément partout. C'est quelque chose à considérer tout en équilibrant les compromis.

Quant à la probabilité d’exiger des changements, cela diminue avec:

1. Bon test (fiabilité améliorée).
2. N'impliquant que le strict minimum nécessaire pour effectuer quelque chose de spécifique (cela peut inclure la réduction des couplages afférents).
3. Rendre le code badass à ce qu'il fait (voir Principe Make Badass).

Quant à la difficulté de faire des changements, elle monte avec des couplages efférents. Les tests introduisent des couplages efférents mais améliorent la fiabilité. Bien fait, il fait généralement plus de bien que de mal et est tout à fait acceptable et promu par le principe du minimum de chagrin.

Assurez le principe de Badass: les classes utilisées dans de nombreux endroits devraient être géniales. Ils doivent être fiables, efficaces si cela est lié à leur qualité, etc.

Et le principe Make Badass est lié au principe Minimum Grief, car les choses badass trouveront une probabilité moins grande d’exiger des changements que les choses qui ne valent que ce qu’elles font.

J'aurais commencé par souligner le paradoxe mentionné ci-dessus, puis indiquer que le PRS dépend fortement du niveau de granularité que vous souhaitez prendre en compte et que, si vous le prenez suffisamment loin, toute classe contenant plus d'une propriété ou une seule méthode viole il.

Du point de vue du SRP, une classe qui fait à peine quoi que ce soit n'aurait certainement qu'une seule raison (parfois zéro) de changer:

class Float
{
public:
    explicit Float(float val);
    float get() const;
    void set(float new_val);
};

Cela n'a pratiquement aucune raison de changer! C'est mieux que SRP. C'est ZRP!

Sauf que je dirais que c'est une violation flagrante du principe Make Badass. C'est aussi absolument nul. Quelque chose qui fait si peu ne peut pas espérer être dur à cuire. Il contient trop peu d'informations (TLI). Et naturellement, lorsque vous avez quelque chose qui est TLI, il ne peut rien faire de vraiment significatif, pas même avec les informations qu’il encapsule, il doit donc le divulguer au monde extérieur dans l’espoir que quelqu'un d'autre fasse réellement quelque chose de significatif et de dur à cuire. Et cette fuite est acceptable pour quelque chose qui est juste destiné à agréger des données et rien de plus, mais ce seuil est la différence que je vois entre "données" et "objets".

Bien sûr, quelque chose qui est TMI est mauvais aussi. Nous pourrions mettre tout notre logiciel dans une classe. Il peut même n’avoir qu’une runméthode. Et quelqu'un pourrait même dire qu'il a maintenant une très grande raison de changer: "Cette classe ne devra être changée que si le logiciel doit être amélioré." Je suis stupide, mais bien sûr, nous pouvons imaginer tous les problèmes de maintenance liés à cela.

Il y a donc un équilibre à trouver dans la granularité ou la grossièreté des objets que vous concevez. Je le juge souvent par la quantité d’informations que vous devez divulguer au monde extérieur et par la quantité de fonctionnalités significatives qu’elle peut exécuter. Je trouve souvent que le principe Make Badass est utile pour trouver l’équilibre tout en le combinant avec le principe Minimum deuil.

Au contraire, pour moi, le modèle de domaine anémique casse certains concepts principaux de POO (qui associent attributs et comportement), mais peut être inévitable en fonction de choix architecturaux. Les domaines anémiques sont plus faciles à penser, moins organiques et plus séquentiels.

De nombreux systèmes ont tendance à le faire lorsque plusieurs couches doivent utiliser les mêmes données (couche de service, couche Web, couche client, agents, etc.).

Il est plus facile de définir la structure de données à un endroit et le comportement dans d'autres classes de service. Si la même classe a été utilisée sur plusieurs couches, celle-ci peut grossir et pose la question de savoir quelle couche est responsable de définir le comportement dont elle a besoin et qui est capable d'appeler les méthodes.

Par exemple, il peut ne pas être une bonne idée qu'un processus d'agent qui calcule des statistiques sur tous vos employés puisse appeler un calcul pour les jours payés. Et l'interface utilisateur graphique de la liste des employés n'a certainement pas besoin du tout du nouveau calcul d'identifiant agrégé utilisé dans cet agent statistique (et des données techniques qui l'accompagnent). Lorsque vous séparez les méthodes de cette manière, vous vous retrouvez généralement avec une classe avec uniquement des structures de données.

Vous pouvez trop facilement sérialiser / désérialiser les données "d'objet", ou juste certaines d'entre elles, ou dans un autre format (JSON) ... sans vous soucier de la notion ou de la responsabilité de l'objet. Ce ne sont que des données qui passent. Vous pouvez toujours faire un mappage de données entre deux classes (Employee, EmployeeVO, EmployeeStatistic…), mais que signifie réellement employé ici?

Donc oui, il sépare complètement les données dans les classes de domaine et le traitement des données dans les classes de service, mais c'est nécessaire. Un tel système est à la fois fonctionnel et technique et permet de propager les données partout où il est nécessaire, tout en gardant un périmètre de responsabilité approprié (et l’objet distribué ne le résout pas non plus).

Si vous n'avez pas besoin de séparer les portées de comportement, vous êtes plus libre de placer les méthodes dans des classes de service ou dans des classes de domaine, en fonction de la façon dont vous voyez votre objet. J'ai tendance à voir un objet comme le "vrai" concept, cela aide naturellement à conserver le SRP. Ainsi, dans votre exemple, il est plus réaliste que le patron de l'employé ajoute un jour de congé accordé à son compte PayDayAccount. Un employé est embauché par la société, Works, peut être malade ou recevoir un conseil, et il dispose d'un compte Payday (le patron peut le récupérer directement auprès de lui ou d'un registre PayDayAccount ...), mais vous pouvez créer un raccourci global. ici pour plus de simplicité si vous ne voulez pas trop de complexité pour un logiciel simple.

Vous enfreignez le principe de responsabilité unique (SRP) car vous représentez des données et exécutez une logique dans la même classe.

Cela semble très raisonnable pour moi. Le modèle peut ne pas avoir de propriétés publiques si expose des actions. Il s’agit essentiellement d’une idée de séparation commande-requête. Veuillez noter que la commande aura un état privé à coup sûr.

Vous ne pouvez pas enfreindre le principe de responsabilité unique, car il ne s'agit que d'un critère esthétique et non d'une règle de la nature. Ne soyez pas induit en erreur par le nom à consonance scientifique et les lettres majuscules.