C# – S.O.L.I.D., avoiding anemic domains, dependency injection

Architecturecdesignnet

Although this could be a programming language agnostic question, I'm interested in answers targeting the .NET ecosystem.

This is the scenario: suppose we need to develop a simple console application for the public administration. The application is about vehicle tax. They (only) have the following business rules:

1.a) If the vehicle is a car and the last time its owner paid the tax was 30 days ago then the owner has to pay again.

1.b) If the vehicle is a motorbike and the last time its owner paid the tax was 60 days ago then the owner has to pay again.

In other words, if you have a car you have to pay every 30 days or if you have a motorbike you have to pay every 60 days.

For each vehicle in the system, the application should test those rules and print those vehicles (plate number and owner information) that don't satisfy them.

What I want is:

2.a) Conform to the S.O.L.I.D. principles (especially Open/closed principle).

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

2.b) An anemic domain, therefore business logic should go inside business entities.

I started with this:

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: The Open/closed principle is guaranteed; if they want bicycle tax you just inherit from Vehicle, then you override the HasToPay method and you're done. Open/closed principle satisfied through simple inheritance.

The "problems" are:

3.a) Why a vehicle has to know if it has to pay? Isn't a PublicAdministration concern? If public administration rules for car tax change, why Car has to change? I think you should be asking: "then why you put a HasToPay method inside Vehicle?", and the answer is: because I don't want to test for the vehicle type (typeof) inside PublicAdministration. Do you see a better alternative?

3.b) Maybe tax payment is a vehicle concern after all, or maybe my initial Person-Vehicle-Car-Motorbike-PublicAdministration design is just plain wrong, or I need a philosopher and a better business analyst. A solution could be: move the HasToPay method to another class, we can call it TaxPayment. Then we create two TaxPayment derived classes; CarTaxPayment and MotorbikeTaxPayment. Then we add an abstract Payment property (of type TaxPayment) to the Vehicle class and we return the right TaxPayment instance from Car and Motorbike classes:

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

And we invoke the old process this way:

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

But now this code won't compile because there's no LastPaidTime member inside CarTaxPayment/MotorbikeTaxPayment/TaxPayment. I'm starting to see CarTaxPayment and MotorbikeTaxPayment more like "algorithm implementations" rather than business entities. Somehow now those "algorithms" need the LastPaidTime value. 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?

3.c) Suppose we already have an Entity Framework ObjectContext (which uses our domain objects; Person, Vehicle, Car, Motorbike, PublicAdministration). How would you do to get an ObjectContext reference from PublicAdministration.GetAllVehicles method and therefore implement the funcionality?

3.d) If we also need the same ObjectContext reference for CarTaxPayment.HasToPay but a different one for MotorbikeTaxPayment.HasToPay, who is in charge of "injecting" or how would you pass those references to the payment classes? In this case, avoiding an anemic domain (and thus no service objects), don't lead us to disaster?

What are your design choices for this simple scenario? Clearly the examples I've shown are "too complex" for an easy task, but at the same time the idea is to adhere to the S.O.L.I.D. principles and prevent anemic domains (of which have been commissioned to destroy).

Best Answer

I would say that neither a person nor a vehicle should know whether a payment is due.

reexamining the problem domain

If you look at the problem domain "people having cars": In order to purchase a car you have some sort of contract which transfers ownership from one person to the other, neither the car nor the person change in that process. Your model is completely lacking that.

thought experiment

Assuming there is a married couple, the man owns the car and he is paying the taxes. Now the man dies, his wife inherits the car and will pay the taxes. Yet, your plates will stay the same (at least in my country they will). It is still the same car! You should be able to model that in your domain.

=> Ownership

A car that is not owned by anyone is still a car but nobody will pay taxes for it. In fact, you are not paying taxes for the car but for the privilege of owning a car. So my proposition would be:

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

}

This is far from perfect and probably not even remotely correct C# code but I hope you get the idea (and somebody could clean this up). The only downside is that you would probably need a factory or something to create the correct CarOwnership between a Car and a Person