S.O.L.I.D., evitando domini anemici, iniezione di dipendenza?

Direi che né una persona né un veicolo dovrebbero sapere se è dovuto un pagamento.

riesame del dominio problematico

Se guardi al dominio del problema "le persone che hanno macchine": per acquistare un'auto hai una specie di contratto che trasferisce la proprietà da una persona all'altra, né la macchina né la persona cambiano in quel processo. Il tuo modello è completamente privo di quello.

esperimento mentale

Supponendo che ci sia una coppia sposata, l'uomo possiede l'auto e paga le tasse. Ora l'uomo muore, sua moglie eredita l'auto e pagherà le tasse. Tuttavia, i tuoi piatti rimarranno gli stessi (almeno nel mio paese lo faranno). È sempre la stessa macchina! Dovresti essere in grado di modellarlo nel tuo dominio.

= > Proprietà

Un'auto che non è di proprietà di nessuno è ancora un'auto, ma nessuno pagherà le tasse per questo. In realtà, non si pagano le tasse per l'auto ma per il privilegio di possedere un'auto . Quindi la mia proposta sarebbe:

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());
    }

}

Questo è ben lungi dall'essere perfetto e probabilmente nemmeno correggere il codice C # da remoto, ma spero che tu abbia ottenuto l'idea (e qualcuno potrebbe ripulirlo). L'unico svantaggio è che probabilmente avrai bisogno di una fabbrica o qualcosa del genere per creare il CarOwnership corretto tra Car e Person

Penso che in questo tipo di situazione, in cui si desidera che le regole aziendali esistano al di fuori degli oggetti di destinazione, il test per il tipo è più che accettabile.

Certamente, in molte situazioni dovresti usare una strategia patern e lasciare che gli oggetti gestiscano le cose da soli, ma non è sempre desiderabile.

Nel mondo reale, un impiegato guarderebbe il tipo di veicolo e controllerebbe i suoi dati fiscali in base a quello. Perché il tuo software non dovrebbe seguire la stessa regola di busiens?

What I don't want is (I think):

2.b) An anemic domain, which means business logic inside business entities.

Hai questo all'indietro. Un dominio anemico è uno che la logica è al di fuori delle entità aziendali. Ci sono molte ragioni per cui l'uso di classi aggiuntive per implementare la logica è una cattiva idea; e solo un paio per il motivo per cui dovresti farlo.

Di qui il motivo per cui si chiama anemico: la classe non ha nulla in esso ..

Che cosa farei:

1. Cambia la tua classe veicolo astratta in un'interfaccia.
2. Aggiungi un'interfaccia ITaxPayment che i veicoli devono implementare. Tieni a portata di mano HasToPay.
3. Rimuovi le classi MotorbikeTaxPayment, CarTaxPayment, TaxPayment. Sono inutili complicazioni / confusione.
4. Ogni tipo di veicolo (auto, bici, camper) dovrebbe implementare al minimo il veicolo e, se sono tassati, dovrebbe implementare anche ITaxPayment. In questo modo puoi delineare tra quei veicoli che non sono tassati e quelli che sono ... Supponendo che questa situazione esista anche.
5. Ogni tipo di veicolo dovrebbe implementare, entro i limiti della classe stessa, i metodi definiti nelle interfacce.
6. Inoltre, aggiungi l'ultima data di pagamento all'interfaccia di ITaxPayment.

Why a vehicle has to know if it has to pay? Isn't a PublicAdministration concern?

No. A quanto ho capito, l'intera app riguarda la tassazione. Quindi la preoccupazione riguardo al fatto che un veicolo debba essere tassato non è più un problema di PublicAdministration, quindi qualsiasi altra cosa nell'intero programma. Non c'è motivo di metterlo altrove ma qui.

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;
    }
}

Questi due frammenti di codice differiscono solo dalla costante. Invece di sovrascrivere l'intera funzione HasToPay () sovrascrive una funzione int DaysBetweenPayments() . Chiamalo invece di usare la costante. Se questo è tutto ciò su cui effettivamente differiscono, lascerei cadere le classi.

Suggerirei anche che Moto / Auto dovrebbe essere una proprietà di categoria del veicolo piuttosto che delle sottoclassi. Questo ti dà più flessibilità. Ad esempio, rende facile cambiare la categoria di una moto o di un'auto. Ovviamente è improbabile che le bici vengano trasformate in auto nella vita reale. Ma tu sai che un veicolo sta per entrare sbagliato e deve essere cambiato in seguito.

Sure, we can pass the value to the constructor of TaxPayment, but that would violate vehicle encapsulation, or responsibilities, or whatever those OOP evangelists call it, wouldn't it?

Non proprio. Un oggetto che passa deliberatamente i suoi dati a un altro oggetto come parametro non è un grosso problema di incapsulamento. La vera preoccupazione sono gli altri oggetti che raggiungono all'interno di questo oggetto per estrarre dati o manipolare i dati all'interno dell'oggetto.

Potresti essere sulla strada giusta. Il problema è, come hai notato, che non c'è nessun membro LastPaidTime all'interno di TaxPayment . Questo è esattamente dove appartiene, anche se non ha bisogno di essere pubblicamente esposto affatto:

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; }
} 

Ogni volta che ricevi un pagamento, crei una nuova istanza di ITaxPayment in base alle norme correnti, che possono avere regole completamente diverse dalla precedente.

Sono d'accordo con la risposta di @sebastiangeiger che il problema riguarda in realtà le proprietà dei veicoli piuttosto che i veicoli. Sto suggerendo di usare la sua risposta ma con alcune modifiche. Vorrei rendere la classe Ownership una classe generica:

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; }
    }
}

E usa l'ereditarietà per specificare l'intervallo di pagamento per ogni tipo di veicolo. Suggerisco inoltre di utilizzare la classe TimeSpan strongmente tipizzata anziché i numeri interi semplici:

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); }
    }
}

Ora il tuo codice effettivo deve solo occuparsi di una 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);
    }
}

Detesto infrangerlo, ma tu hai un dominio anemico , cioè la logica di business al di fuori degli oggetti. Se questo è quello che stai andando, va bene, ma dovresti leggere dei motivi per evitarlo.

Cerca di non modellare il dominio del problema, ma modella la soluzione. Ecco perché sei finito con gerarchie di ereditarietà parallele e più complessità del necessario.

Qualcosa del genere è tutto ciò che ti serve per questo esempio:

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)
}

Se vuoi seguire SOLID che è grandioso, ma come dice lo stesso Zio Bob, sono solo dei principi di ingegneria. Non sono pensati per essere applicati con rigore, ma sono linee guida che dovrebbero essere considerate.

Penso che tu abbia ragione sul fatto che il periodo di pagamento non è una proprietà dell'attuale Auto / Moto. Ma è un attributo della classe di veicolo.

Mentre si può andare sulla strada di avere un if / select sul tipo di oggetto, questo non è molto scalabile. Se successivamente aggiungi camion e Segway e spinga bici, autobus e aereo (può sembrare improbabile, ma non sai mai con i servizi pubblici), la condizione diventa più grande. E se vuoi cambiare le regole per un camion, devi trovarlo in quella lista.

Quindi, perché non renderlo, letteralmente, un attributo e usare la riflessione per estrarlo? Qualcosa del genere:

[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;
    }
}

Potresti anche andare con una variabile statica, se questo è più comodo.

Gli svantaggi sono

• che sarà leggermente più lento, e presumo che tu debba elaborare molti veicoli. Se questo è un problema, potrei prendere una visione più pragmatica e attenermi alla proprietà astratta o all'approccio interfacciato. (Anche se considererei il caching per tipo.)

• Dovrai convalidare all'avvio che ciascuna sottoclasse di veicolo ha un attributo IPaymentRequiredCalculator (o consentire un default), perché non vuoi che elabori 1000 auto, solo per bloccarsi perché la moto non ha un PaymentPeriod.

Il lato positivo è che se in un secondo momento incontri un mese di calendario o un pagamento annuale, puoi semplicemente scrivere una nuova classe di attributi. Questa è la definizione stessa di OCP.