Comment argumenter contre cet état d'esprit «complètement public» de la conception de classes d'objets métier

Nous faisons beaucoup de tests unitaires et de refactorisation de nos objets métier, et je semble avoir des opinions très différentes sur la conception des classes que les autres pairs.

Un exemple de cours dont je ne suis pas fan:

public class Foo
{
    private string field1;
    private string field2;
    private string field3;
    private string field4;
    private string field5;

    public Foo() { }

    public Foo(string in1, string in2)
    {
        field1 = in1;
        field2 = in2;
    }

    public Foo(string in1, string in2, string in3, string in4)
    {
        field1 = in1;
        field2 = in2;
        field3 = in3;
    }

    public Prop1
    { get { return field1; } }
    { set { field1 = value; } }

    public Prop2
    { get { return field2; } }
    { set { field2 = value; } }

    public Prop3
    { get { return field3; } }
    { set { field3 = value; } }

    public Prop4
    { get { return field4; } }
    { set { field4 = value; } }

    public Prop5
    { get { return field5; } }
    { set { field5 = value; } }

}

Dans la classe "réelle", elles ne sont pas toutes des chaînes, mais dans certains cas, nous avons 30 champs de support pour les propriétés complètement publiques.

Je déteste ce cours et je ne sais pas si je suis juste difficile. Quelques points importants:

• Champs de sauvegarde privés sans logique dans les propriétés, semble inutile et gonfle la classe
• Constructeurs multiples (assez bien) mais couplés à
• toutes les propriétés ayant un setter public, je ne suis pas fan.
• Aucune propriété ne serait potentiellement affectée à une valeur en raison du constructeur vide, si un appelant n'est pas au courant, vous pourriez potentiellement obtenir des comportements très indésirables et difficiles à tester.
• C'est trop de propriétés! (dans le cas 30)

Je trouve beaucoup plus difficile de vraiment savoir dans quel état se trouve l'objet Fooà un moment donné, en tant qu'implémenteur. L'argument a été avancé: «nous pourrions ne pas avoir les informations nécessaires à définir Prop5au moment de la construction de l'objet. D'accord, je suppose que je peux le comprendre, mais si c'est le cas, ne rendre Prop5public que le setter, pas toujours jusqu'à 30 propriétés dans une classe.

Suis-je juste pointilleux et / ou fou pour vouloir une classe qui est "facile à utiliser" par opposition à "facile à écrire (tout public)"? Des cours comme ceux ci-dessus me crient, je ne sais pas comment ça va être utilisé, donc je vais juste rendre tout public juste au cas où .

Si je ne suis pas très difficile, quels sont les bons arguments pour lutter contre ce type de pensée? Je ne suis pas très bon pour articuler des arguments, car je suis très frustré d'essayer de faire passer mon message (pas intentionnellement bien sûr).

Les classes entièrement publiques ont une justification pour certaines situations, ainsi que pour les autres classes extrêmes, avec une seule méthode publique (et probablement beaucoup de méthodes privées). Et des cours avec des méthodes publiques et privées aussi.

Tout dépend du type d'abstraction que vous allez modéliser avec eux, des couches que vous avez dans votre système, du degré d'encapsulation dont vous avez besoin dans les différentes couches et (bien sûr) de quelle école de pensée l'auteur de la classe vient de. Vous pouvez trouver tous ces types dans le code SOLID.

Il y a des livres entiers écrits sur le moment de préférer quel type de conception, donc je ne vais pas énumérer ici de règles à ce sujet, l'espace dans cette section ne serait pas suffisant. Cependant, si vous avez un exemple réel d'une abstraction que vous aimez modéliser avec une classe, je suis sûr que la communauté ici vous aidera avec plaisir à améliorer la conception.

Pour aborder vos autres points:

• "champs de support privés sans logique dans les propriétés": Oui, vous avez raison, pour les getters et les setters triviaux, c'est juste un "bruit" inutile. Pour éviter ce type de "ballonnement", C # a une syntaxe de raccourci pour les méthodes get / set de propriété:

Donc au lieu de

   private string field1;
   public string Prop1
   { get { return field1; } }
   { set { field1 = value; } }

écrire

   public string Prop1 { get;set;}

ou

   public string Prop1 { get;private set;}

• "Constructeurs multiples": ce n'est pas un problème en soi. Il y a un problème lorsqu'il y a duplication de code inutile, comme indiqué dans votre exemple, ou lorsque la hiérarchie des appels est compliquée. Cela peut être facilement résolu en refactorisant les parties communes dans une fonction distincte et en organisant la chaîne du constructeur de manière unidirectionnelle

• "Aucune propriété ne serait potentiellement affectée à cause du constructeur vide": en C #, chaque type de données a une valeur par défaut clairement définie. Si les propriétés ne sont pas initialisées explicitement dans un constructeur, elles obtiennent cette valeur par défaut affectée. Si cela est utilisé intentionnellement , c'est tout à fait correct - donc un constructeur vide pourrait être correct si l'auteur sait ce qu'il fait.

• "C'est trop de propriétés! (Dans le cas 30)": oui, si vous êtes libre de concevoir une telle classe de manière entièrement nouvelle, 30 sont trop, je suis d'accord. Cependant, tout le monde n'a pas ce luxe (n'avez-vous pas écrit dans le commentaire ci-dessous qu'il s'agit d'un système hérité?). Parfois, vous devez mapper des enregistrements d'une base de données ou d'un fichier existant, ou des données d'une API tierce à votre système. Donc, pour ces cas, 30 attributs peuvent être quelque chose avec lequel on doit vivre.

1. En disant que "les champs de sauvegarde privés sans logique dans les propriétés, semblent inutiles et gonflent la classe", vous avez déjà un argument décent.

2. Il ne semble pas que plusieurs constructeurs soient en cause ici.

3. Quant à avoir toutes les propriétés publiques ... Peut-être pensez-vous de cette façon, si vous avez toujours voulu synchroniser l'accès à toutes ces propriétés entre les threads, vous auriez du mal car vous pourriez avoir à écrire du code de synchronisation partout où ils sont utilisé. Cependant, si toutes les propriétés étaient enfermées par des getters / setters, vous pourriez facilement créer la synchronisation dans la classe.

4. Je pense que lorsque vous dites "comportement difficile à tester", l'argument parle de lui-même. Votre test devra peut-être vérifier s'il n'est pas nul ou quelque chose comme ça (chaque endroit où la propriété est utilisée). Je ne sais pas vraiment parce que je ne sais pas à quoi ressemble votre test / application.

5. Vous avez bien trop de propriétés, pourriez-vous essayer d'utiliser l'héritage (si les propriétés sont assez similaires) et ensuite avoir une liste / tableau utilisant des génériques? Ensuite, vous pouvez écrire des getters / setters pour accéder aux informations et simplifier la façon dont vous interagirez avec toutes les propriétés.

Comme vous me l'avez dit, Fooc'est un objet commercial. Il doit encapsuler certains comportements dans les méthodes en fonction de votre domaine et ses données doivent rester aussi encapsulées que possible.

Dans l'exemple que vous montrez, Fooressemble plus à un DTO et va à l'encontre de tous les principes de la POO (voir Modèle de domaine anémique )!

Comme améliorations, je suggère:

• Rendez cette classe aussi immuable que possible. Comme vous l'avez dit, vous voulez faire des tests unitaires. Le déterminisme est une capacité obligatoire pour les tests unitaires et le déterminisme peut être atteint via l'immuabilité car il résout les problèmes d' effets secondaires .
• Divisez cette classe divine en plusieurs classes qui ne font qu'une seule chose chacune (voir SRP ). Test unitaire d'une classe de 30 attributs dans vraiment un PITA .
• Supprimez la redondance des constructeurs en n'ayant qu'un seul constructeur principal et les autres appelant le constructeur principal.
• Supprimez les getters inutiles, je doute sérieusement que tous les getters soient vraiment utiles.
• Ramenez la logique métier dans les classes affaires, c'est à ça qu'il appartient!

Cela pourrait être une classe refactorisée potentielle avec ces remarques:

public sealed class Foo
{
    private readonly string field1;
    private readonly string field2;
    private readonly string field3;
    private readonly string field4;

    public Foo() : this("default1", "default2")
    { }

    public Foo(string in1, string in2) : this(in1, in2, "default3", "default4")
    { }

    public Foo(string in1, string in2, string in3, string in4)
    {
        field1 = in1;
        field2 = in2;
        field3 = in3;
        field4 = in4;
    }

    //Methods with business logic

    //Really needed ?
    public Prop1
    { get; }

    //Really needed ?
    public Prop2
    { get; }

    //Really needed ?
    public Prop3
    { get; }

    //Really needed ?
    public Prop4
    { get; }

    //Really needed ?
    public Prop5
    { get; }
}

Comment argumenter contre cet état d'esprit «complètement public» de la conception de classes d'objets métier

• "Il existe plusieurs méthodes dispersées dans la ou les classes de clients qui font plus que de simples" tâches DTO ". Elles vont de pair."
• "Il peut s'agir d'un" DTO "mais il a une identité commerciale - nous devons remplacer Equals."
• "Nous devons les trier - nous devons implémenter IComparable"
• "Nous enchaînons plusieurs de ces propriétés. Remplaçons ToString pour que chaque client n'ait pas à écrire ce code."
• "Nous avons plusieurs clients qui font la même chose avec cette classe. Nous devons SECHER le code."
• "Le client manipule une collection de ces choses. Il est évident qu'il devrait s'agir d'une classe de collection personnalisée; et bon nombre des points précédents rendront cette collection personnalisée plus fonctionnelle qu'aujourd'hui."
• "Regardez toutes les manipulations fastidieuses et exposées des chaînes! Encapsuler cela dans les méthodes commerciales augmentera notre productivité de codage."
• "Le fait de rassembler ces méthodes dans la classe les rendra testables car nous n'avons pas à gérer la classe cliente la plus complexe."

• "Nous pouvons maintenant tester unitairement ces méthodes refactorisées. En l'état, pour des raisons x, y, z, le client n'est pas testable.

• Dans la mesure où l'un des arguments ci-dessus peut être avancé, dans l'ensemble:

  • La fonctionnalité devient réutilisable.
  • C'est sec
  • Il est découplé des clients.
  • le codage par rapport à la classe est plus rapide et moins sujet aux erreurs.
• "Une classe bien faite masque l'état et expose les fonctionnalités. C'est la proposition de départ pour concevoir des classes OO."
• "Le résultat final fait un bien meilleur travail d'expression du domaine des affaires; les entités distinctes et leur interaction explicite."
• "Cette fonctionnalité" indirecte "(c'est-à-dire pas d'exigences fonctionnelles explicites, mais cela doit être fait) se démarque clairement et adhère au principe de responsabilité unique.