SOLIDE, éviter les domaines anémiques, injection de dépendance?

33

Bien que cela puisse être une question agnostique en langage de programmation, je suis intéressé par les réponses ciblant l'écosystème .NET.

Voici le scénario: supposons que nous devions développer une application console simple pour l’administration publique. L'application concerne la taxe sur les véhicules. Ils ont (uniquement) les règles commerciales suivantes:

1.a) Si le véhicule est une voiture et que le dernier paiement de la taxe par son propriétaire remonte à 30 jours, il doit alors payer à nouveau.

1.b) Si le véhicule est une moto et que le dernier paiement de la taxe par son propriétaire a eu lieu il y a 60 jours, il doit payer à nouveau.

En d'autres termes, si vous avez une voiture, vous devez payer tous les 30 jours ou si vous avez une moto, vous devez payer tous les 60 jours.

L'application doit tester ces règles pour chaque véhicule du système et imprimer les véhicules (numéro de plaque et informations sur le propriétaire) qui ne les satisfont pas.

Ce que je veux c'est:

2.a) Conforme aux principes SOLID (notamment principe ouvert / fermé).

Ce que je ne veux pas, c'est (je pense):

2.b) Un domaine anémique, la logique métier doit donc être intégrée aux entités métier.

J'ai commencé avec ça:

public class Person
// You wanted a banana but what you got was a gorilla holding the banana and the entire jungle.
{
    public string Name { get; set; }

    public string Surname { get; set; }
}

public abstract class Vehicle
{
    public string PlateNumber { get; set; }

    public Person Owner { get; set; }

    public DateTime LastPaidTime { get; set; }

    public abstract bool HasToPay();
}

public class Car : Vehicle
{
    public override bool HasToPay()
    {
        return (DateTime.Today - this.LastPaidTime).TotalDays >= 30;
    }
}

public class Motorbike : Vehicle
{
    public override bool HasToPay()
    {
        return (DateTime.Today - this.LastPaidTime).TotalDays >= 60;
    }
}

public class PublicAdministration
{
    public IEnumerable<Vehicle> GetVehiclesThatHaveToPay()
    {
        return this.GetAllVehicles().Where(vehicle => vehicle.HasToPay());
    }

    private IEnumerable<Vehicle> GetAllVehicles()
    {
        throw new NotImplementedException();
    }
}

class Program
{
    static void Main(string[] args)
    {
        PublicAdministration administration = new PublicAdministration();
        foreach (var vehicle in administration.GetVehiclesThatHaveToPay())
        {
            Console.WriteLine("Plate number: {0}\tOwner: {1}, {2}", vehicle.PlateNumber, vehicle.Owner.Surname, vehicle.Owner.Name);
        }
    }
}

2.a: le principe d'ouverture / fermeture est garanti; s'ils veulent une taxe sur le vélo dont vous héritez simplement de Vehicle, vous remplacez la méthode HasToPay et vous avez terminé. Principe ouvert / fermé satisfait par un héritage simple.

Les "problèmes" sont:

3.a) Pourquoi un véhicule doit-il savoir s'il doit payer? N'est-ce pas un problème d'administration publique? Si l'administration publique décide de modifier la taxe sur les voitures, pourquoi Car doit-elle changer? Je pense que vous devriez vous demander: "alors pourquoi vous mettez une méthode HasToPay dans Vehicle?", Et la réponse est: parce que je ne veux pas tester le type de véhicule (typeof) dans PublicAdministration. Voyez-vous une meilleure alternative?

3.b) Après tout, le paiement des taxes est peut-être une affaire de véhicules, ou peut-être que mon concept initial de personne-véhicule-voiture-moto-administration publique est tout simplement faux, ou j'ai besoin d'un philosophe et d'un meilleur analyste commercial. Une solution pourrait être: déplacer la méthode HasToPay dans une autre classe, nous pouvons l'appeler TaxPayment. Ensuite, nous créons deux classes dérivées de TaxPayment; CarTaxPayment et MotorbikeTaxPayment. Ensuite, nous ajoutons une propriété de paiement abstraite (de type TaxPayment) à la classe Vehicle et nous renvoyons la bonne instance TaxPayment des classes Car et Moto:

public abstract class TaxPayment
{
    public abstract bool HasToPay();
}

public class CarTaxPayment : TaxPayment
{
    public override bool HasToPay()
    {
        return (DateTime.Today - this.LastPaidTime).TotalDays >= 30;
    }
}

public class MotorbikeTaxPayment : TaxPayment
{
    public override bool HasToPay()
    {
        return (DateTime.Today - this.LastPaidTime).TotalDays >= 60;
    }
}

public abstract class Vehicle
{
    public string PlateNumber { get; set; }

    public Person Owner { get; set; }

    public DateTime LastPaidTime { get; set; }

    public abstract TaxPayment Payment { get; }
}

public class Car : Vehicle
{
    private CarTaxPayment payment = new CarTaxPayment();
    public override TaxPayment Payment
    {
        get { return this.payment; }
    }
}

public class Motorbike : Vehicle
{
    private MotorbikeTaxPayment payment = new MotorbikeTaxPayment();
    public override TaxPayment Payment
    {
        get { return this.payment; }
    }
}

Et nous invoquons l'ancien processus de cette façon:

    public IEnumerable<Vehicle> GetVehiclesThatHaveToPay()
    {
        return this.GetAllVehicles().Where(vehicle => vehicle.Payment.HasToPay());
    }

Mais maintenant, ce code ne sera pas compilé car il n'y a pas de membre LastPaidTime dans CarTaxPayment / MotorbikeTaxPayment / TaxPayment. Je commence à voir CarTaxPayment et MotorbikeTaxPayment plus comme des "implémentations d'algorithmes" plutôt que des entités commerciales. D'une manière ou d'une autre, ces "algorithmes" ont besoin de la valeur LastPaidTime. Bien sûr, nous pouvons transmettre la valeur au constructeur de TaxPayment, mais cela violerait l’encapsulation du véhicule, ou les responsabilités, ou peu importe ce que ces évangélistes de la POO appellent, non?

3.c) Supposons que nous ayons déjà un ObjectContext Entity Framework (qui utilise nos objets de domaine: Personne, Véhicule, Voiture, Moto, Administration publique). Comment feriez-vous pour obtenir une référence ObjectContext à partir de la méthode PublicAdministration.GetAllVehicles et donc implémenter la fonctionnalité?

3.d) Si nous avons également besoin de la même référence ObjectContext pour CarTaxPayment.HasToPay mais d'une valeur différente pour MotorbikeTaxPayment.HasToPay, qui est chargé de "l'injection" ou comment transmettriez-vous ces références aux classes de paiement? Dans ce cas, éviter un domaine anémique (et donc pas d'objets de service), ne nous mène pas au désastre?

Quels sont vos choix de conception pour ce scénario simple? Il est clair que les exemples que j'ai présentés sont "trop ​​complexes" pour une tâche facile, mais en même temps, l'idée est d'adhérer aux principes SOLID et de prévenir les domaines anémiques (dont la destruction a été commandée).

David Robert Jones
la source

Réponses:

15

Je dirais que ni une personne ni un véhicule ne doivent savoir si un paiement est dû.

réexaminer le domaine problématique

Si vous regardez le domaine problématique "personnes ayant des voitures": Pour acheter une voiture, vous avez une sorte de contrat qui transfère la propriété d'une personne à l'autre, ni la voiture ni la personne ne changent dans ce processus. Votre modèle manque complètement de cela.

expérience de pensée

En supposant qu'il y ait un couple marié, l'homme est propriétaire de la voiture et il paie les taxes. Maintenant, l'homme meurt, sa femme hérite de la voiture et paiera les taxes. Pourtant, vos assiettes resteront les mêmes (du moins dans mon pays, elles le seront). C'est toujours la même voiture! Vous devriez pouvoir modéliser cela dans votre domaine.

=> Propriété

Une voiture qui n'appartient à personne est toujours une voiture mais personne ne paiera d'impôts pour l'avoir. En fait, vous ne payez pas d'impôts pour la voiture, mais pour le privilège de posséder une voiture . Donc, ma proposition serait:

public class Person
{
    public string Name { get; set; }

    public string Surname { get; set; }

    public List<Ownership> getOwnerships();

    public List<Vehicle> getVehiclesOwned();
}

public abstract class Vehicle
{
    public string PlateNumber { get; set; }

    public Ownership currentOwnership { get; set; }

}

public class Car : Vehicle {}

public class Motorbike : Vehicle {}

public abstract class Ownership
{
    public Ownership(Vehicle vehicle, Owner owner);

    public Vehicle Vehicle { get;}

    public Owner CurrentOwner { get; set; }

    public abstract bool HasToPay();

    public DateTime LastPaidTime { get; set; }

   //Could model things like payment history or owner history here as well
}

public class CarOwnership : Ownership
{
    public override bool HasToPay()
    {
        return (DateTime.Today - this.LastPaidTime).TotalDays >= 30;
    }
}

public class MotorbikeOwnership : Ownership
{
    public override bool HasToPay()
    {
        return (DateTime.Today - this.LastPaidTime).TotalDays >= 60;
    }
}

public class PublicAdministration
{
    public IEnumerable<Ownership> GetVehiclesThatHaveToPay()
    {
        return this.GetAllOwnerships().Where(HasToPay());
    }

}

C’est loin d’être parfait et probablement même pas du code C # correct à distance, mais j’espère que vous aurez l’idée (et que quelqu'un pourrait le nettoyer). Le seul inconvénient est que vous auriez probablement besoin d' une usine ou quelque chose pour créer le bon CarOwnershipentre un CaretPerson

sebastiangeiger
la source
Je pense que le véhicule lui-même devrait enregistrer les taxes payées et les particuliers devraient enregistrer les taxes payées pour tous leurs véhicules. Un code de taxe pourrait être écrit de sorte que si la taxe sur une voiture est payée le 1er juin et vendue le 10 juin, le nouveau propriétaire pourrait en être redevable le 10 juin, le 1er juillet ou le 10 juillet (et même si c'est le cas). actuellement, cela pourrait changer). Si la règle des 30 jours est constante pour la voiture, même en cas de changement de propriétaire, mais que les voitures dues par personne ne peuvent pas payer d'impôts, alors je suggérerais que lorsqu'une voiture qui n'avait pas été détenue depuis 30 jours ou plus est achetée, paiement de l'impôt ...
Supercat
... enregistré d'une personne fictive du "crédit d'impôt pour véhicule non possédé", de sorte qu'à compter du moment de la vente, la taxe sera redevable de zéro ou de trente jours, quelle que soit la durée pendant laquelle le véhicule n'a pas été possédé.
Supercat
4

Je pense que dans ce type de situation, où vous souhaitez que les règles métier existent en dehors des objets cibles, le test du type est plus qu'acceptable.

Certes, dans de nombreuses situations, vous devez utiliser un modèle de stratégie et laisser les objets gérer les choses eux-mêmes, mais ce n'est pas toujours souhaitable.

Dans le monde réel, un greffier examinerait le type de véhicule et consulterait ses données fiscales sur cette base. Pourquoi votre logiciel ne devrait-il pas suivre la même règle de busienss?

Erik Funkenbusch
la source
Ok, disons que vous m'avez convaincu, et de la méthode GetVehiclesThatHaveToPay, nous inspectons le type de véhicule. Qui devrait être responsable de LastPaidTime? LastPaidTime est-il une affaire de véhicule ou de public administration? Parce que si c'est un véhicule, cette logique commerciale ne devrait-elle pas résider dans Vehicle? Que peux-tu me dire sur la question 3.c?
@DavidRobertJones - Idéalement, je rechercherais le dernier paiement dans l'historique des paiements, en fonction de la licence du véhicule. Un paiement de taxe est une préoccupation fiscale, pas une préoccupation de véhicule.
Erik Funkenbusch
5
@ David RavertJones Je pense que vous vous trompez avec votre propre métaphore. Ce que vous avez n’est pas un véhicule qui doit faire tout ce qu’il fait. Au lieu de cela, vous avez nommé, pour simplifier, un véhicule taxable (ou quelque chose de similaire) un véhicule. Votre domaine entier est le paiement des taxes. Ne vous inquiétez pas de ce que font les véhicules. Et, si nécessaire, abandonnez la métaphore et proposez-en une plus appropriée.
3

Ce que je ne veux pas, c'est (je pense):

2.b) Un domaine anémique, ce qui signifie la logique métier au sein des entités métier.

Vous avez cela à l'envers. Un domaine anémique est un domaine dans lequel la logique est en dehors des entités commerciales. Il y a de nombreuses raisons pour lesquelles utiliser des classes supplémentaires pour implémenter la logique est une mauvaise idée. et seulement un couple pour pourquoi vous devriez le faire.

D'où la raison pour laquelle cela s'appelle anémique: la classe n'a rien dedans ..

Ce que je ferais:

  1. Changez votre classe abstraite Vehicle en interface.
  2. Ajoutez une interface ITaxPayment que les véhicules doivent implémenter. Demandez à votre HasToPay de raccrocher d'ici.
  3. Supprimez les classes MotorbikeTaxPayment, CarTaxPayment, TaxPayment. Ce sont des complications inutiles / fouillis.
  4. Chaque type de véhicule (voiture, vélo, camping-car) doit implémenter au minimum un véhicule et, s’ils sont taxés, implémenter également ITaxPayment. De cette façon, vous pouvez distinguer entre les véhicules qui ne sont pas taxés et ceux qui le sont ... En supposant que cette situation existe même.
  5. Chaque type de véhicule doit implémenter, dans les limites de la classe elle-même, les méthodes définies dans vos interfaces.
  6. Ajoutez également la date du dernier paiement à votre interface ITaxPayment.
Pas moi
la source
Tu as raison. A l'origine, la question n'était pas formulée de cette façon, j'ai supprimé une partie et le sens a changé. C'est corrigé maintenant. Merci!
2

Pourquoi un véhicule doit-il savoir s'il doit payer? N'est-ce pas un problème d'administration publique?

Non. Si je comprends bien, toute votre application concerne la taxation. La question de savoir si un véhicule doit être taxé ne concerne donc plus l'administration publique, ni rien d'autre dans l'ensemble du programme. Il n'y a aucune raison de le mettre ailleurs que chez nous.

public class Car : Vehicle
{
    public override bool HasToPay()
    {
        return (DateTime.Today - this.LastPaidTime).TotalDays >= 30;
    }
}

public class Motorbike : Vehicle
{
    public override bool HasToPay()
    {
        return (DateTime.Today - this.LastPaidTime).TotalDays >= 60;
    }
}

Ces deux extraits de code ne diffèrent que par la constante. Au lieu de remplacer la totalité de la fonction HasToPay (), remplacez une int DaysBetweenPayments()fonction. Appelez cela au lieu d'utiliser la constante. Si c'est tout ce sur quoi ils diffèrent, je laisserais tomber les cours.

Je suggérerais également que Moto / Voiture devrait être une propriété de catégorie de véhicule plutôt que de sous-classes. Cela vous donne plus de flexibilité. Par exemple, il est facile de changer de catégorie de moto ou de voiture. Bien sûr, il est peu probable que les vélos soient transformés en voitures dans la vie réelle. Mais vous savez qu'un véhicule va être mal entré et doit être changé plus tard.

Bien sûr, nous pouvons transmettre la valeur au constructeur de TaxPayment, mais cela violerait l’encapsulation du véhicule, ou les responsabilités, ou peu importe ce que ces évangélistes de la POO appellent, non?

Pas vraiment. Un objet qui transmet délibérément ses données à un autre objet en tant que paramètre n'est pas un gros problème d'encapsulation. La véritable préoccupation concerne d'autres objets qui atteignent l'intérieur de cet objet pour extraire des données ou manipuler les données à l'intérieur de l'objet.

Winston Ewert
la source
1

Vous êtes peut-être sur la bonne voie. Le problème est, comme vous l'avez remarqué, qu'il n'y a pas de membre LastPaidTime dans TaxPayment . C'est exactement ce à quoi il appartient, même s'il n'a pas besoin d'être exposé publiquement du tout:

public interface ITaxPayment
{
    // Your base TaxPayment class will know the 
    // next due date. But you can easily create
    // one which uses (for example) fixed dates
    bool IsPaymentDue();
}

public interface IVehicle
{
    string Plate { get; }
    Person Owner { get; }
    ITaxPayment LastTaxPayment { get; }
} 

Chaque fois que vous recevez un paiement, vous créez une nouvelle instance de ITaxPaymentselon les règles en vigueur, qui peuvent avoir des règles complètement différentes de la précédente.

Groo
la source
Le fait est que le paiement dû dépend de la dernière fois que le propriétaire a payé (véhicules différents, dates d'échéance différentes). Comment ITaxPayment sait quelle a été la dernière fois qu'un véhicule (ou son propriétaire) a payé pour effectuer le calcul?
1

Je souscris à la réponse de @sebastiangeiger selon laquelle le problème concerne réellement les propriétaires de véhicules plutôt que les véhicules. Je vais suggérer d'utiliser sa réponse, mais avec quelques changements. Je ferais de la Ownershipclasse une classe générique:

public class Vehicle {}

public abstract class Ownership<T>
{
    public T Item { get; set; }

    public DateTime LastPaidTime { get; set; }

    public abstract TimeSpan PaymentInterval { get; }

    public virtual bool HasToPay
    {
        get { return (DateTime.Today - this.LastPaidTime) >= this.PaymentInterval; }
    }
}

Et utilisez héritage pour spécifier l'intervalle de paiement pour chaque type de véhicule. Je suggère également d'utiliser la TimeSpanclasse fortement typée au lieu des entiers simples:

public class CarOwnership : Ownership<Vehicle>
{
    public override TimeSpan PaymentInterval
    {
        get { return new TimeSpan(30, 0, 0, 0); }
    }
}

public class MotorbikeOwnership : Ownership<Vehicle>
{
    public override TimeSpan PaymentInterval
    {
        get { return new TimeSpan(60, 0, 0, 0); }
    }
}

Maintenant, votre code réel doit seulement traiter avec une classe - Ownership<Vehicle>.

public class PublicAdministration
{
    List<Ownership<Vehicle>> VehicleOwnerships { get; set; }

    public IEnumerable<Vehicle> GetVehiclesThatHaveToPay()
    {
        return VehicleOwnerships.Where(x => x.HasToPay).Select(x => x.Item);
    }
}
utilisateur74130
la source
0

Je n'aime pas vous en parler, mais vous avez un domaine anémique , c'est-à-dire la logique métier en dehors des objets. Si c'est ce que vous allez faire, c'est bien, mais vous devriez lire les raisons pour en éviter un.

Essayez de ne pas modéliser le domaine problématique, mais de modéliser la solution. C'est pourquoi vous vous retrouvez avec des hiérarchies d'héritage parallèles et avec plus de complexité que nécessaire.

Quelque chose comme ceci est tout ce dont vous avez besoin pour cet exemple:

public class Vehicle
{
    public Boolean isMotorBike()
    public string PlateNumber { get; set; }
    public String Owner { get; set; }
    public DateTime LastPaidTime { get; set; }
}

public class TaxPaymentCalculator
{
    public List<Vehicle> GetVehiclesThatHaveToPay(List<Vehicle> all)
}

Si vous voulez suivre SOLID, c’est génial, mais comme l’oncle Bob l’a dit lui-même, ce ne sont que des principes techniques . Elles ne sont pas censées être appliquées de manière stricte, mais constituent des lignes directrices à prendre en compte.

Garrett Hall
la source
4
Je pense que votre solution n'est pas particulièrement évolutive. Vous devrez ajouter un nouveau booléen pour chaque type de véhicule que vous ajoutez. C'est un mauvais choix à mon avis.
Erik Funkenbusch
@Garrett Hall, j'ai bien aimé que vous supprimiez la classe Person de mon "domaine". Je n'aurais pas ce cours en premier lieu, donc désolé pour ça. Mais isMotorBike n'a pas de sens. Quelle serait la mise en œuvre?
1
@DavidRobertJones: Je suis d'accord avec Mystere, l'ajout d'une isMotorBikeméthode n'est pas un bon choix de conception POO. C'est comme remplacer le polymorphisme par un code de type (bien qu'un code booléen).
Groo
@MystereMan, on pourrait facilement laisser tomber les bools et utiliser unenum Vehicles
radarbob
+1 Essayez de ne pas modéliser le domaine problématique, mais de modéliser la solution . Pithy . Mémorable dit. ET les principes commentent. Je vois trop d’essais d’interprétation littérale stricte ; absurdité.
radarbob
0

Je pense que vous avez raison, le délai de paiement n’est pas la propriété de la voiture / moto. Mais c'est un attribut de la classe de véhicule.

Bien que vous puissiez choisir d'avoir un if / select sur le type d'objet, cela n'est pas très évolutif. Si vous ajoutez plus tard un camion et un Segway et que vous poussez un vélo, un bus et un avion (cela peut sembler improbable, mais vous ne le savez jamais avec les services publics), la situation s'aggrave. Et si vous voulez changer les règles pour un camion, vous devez le trouver dans cette liste.

Alors, pourquoi ne pas en faire, littéralement, un attribut et utiliser la réflexion pour le retirer? Quelque chose comme ça:

[DaysBetweenPayments(30)]
public class Car : Vehicle
{
    // actual properties of the physical vehicle
}

[AttributeUsage(AttributeTargets.Class, Inherited = false)]
public class DaysBetweenPaymentsAttribute : Attribute, IPaymentRequiredCalculator
{
    private int _days;

    public DaysBetweenPaymentAttribute(int days)
    {
        _days = days;
    }

    public bool HasToPay(Vehicle vehicle)
    {
        return vehicle.LastPaid.AddDays(_days) <= DateTime.Now;
    }
}

Vous pouvez également utiliser une variable statique, si cela est plus confortable.

Les inconvénients sont

  • que ce sera un peu plus lent, et je suppose que vous devez traiter beaucoup de véhicules. Si cela pose un problème, je pourrais adopter une vision plus pragmatique et rester fidèle à la propriété abstraite ou à l’interface. (Bien que, je considérerais aussi la mise en cache par type.)

  • Vous devrez valider au démarrage que chaque sous-classe Vehicle a un attribut IPaymentRequiredCalculator (ou autorisez une valeur par défaut), car vous ne souhaitez pas que 1000 voitures soient traitées, mais uniquement en panne, Motorbike ne disposant pas de PaymentPeriod.

L'avantage est que si vous rencontrez plus tard un mois de calendrier ou un paiement annuel, vous pouvez simplement écrire une nouvelle classe d'attribut. C’est la définition même de OCP.

pdr
la source
Le problème majeur avec cela est que si vous souhaitez modifier le nombre de jours entre les paiements d'un véhicule, vous devez le reconstruire et le redéployer, et si la fréquence de paiement varie en fonction d'une propriété de l'entité (par exemple, la taille du moteur), difficile d'obtenir une solution élégante pour cela.
Ed James
@ EdWoodcock: Je pense que le premier problème est vrai pour la plupart des solutions et que cela ne m'inquiéterait vraiment pas. S'ils veulent ce niveau de flexibilité plus tard, je leur donnerais une application de base de données. Sur le deuxième point, je suis complètement en désaccord. À ce stade, vous ajoutez simplement véhicule en tant qu'argument facultatif à HasToPay () et vous l'ignorez là où vous n'en avez pas besoin.
pdr
Je suppose que cela dépend des projets à long terme que vous avez pour l'application, ainsi que des exigences particulières du client, quant à savoir s'il est utile de brancher une base de données si aucune n'existe déjà (j'ai tendance à supposer que presque tous les logiciels sont des bases de données -driven, ce que je sais n'est pas toujours le cas). Cependant, sur le deuxième point, le qualificatif important est élégant ; Je ne dirais pas que l'ajout d'un paramètre supplémentaire à une méthode sur un attribut qui prend ensuite une instance de la classe à laquelle l'attribut est attaché est une solution particulièrement élégante. Mais encore une fois, cela dépend des besoins de vos clients.
Ed James
@ EdWoodcock: En fait, je pensais justement à cela et l'argument de LastPaid devra déjà être une propriété de Vehicle. Compte tenu de votre commentaire, il serait peut-être intéressant de remplacer cet argument maintenant plutôt que plus tard - le modifiera.
pdr