Refactoring domain logic that accesses repositories in a legacy system

Question

I am working with a legacy system that has an anemic domain model.

The domain has the following entity classses: Car, CarType, CarComponent, CarComponentType.

For each of these, there is a separate repository. There is also a number of services that access these repositories and contain basically all logic.

I need to implement a method that determines if a CarComponentType can be discontinued by the vendor. The logic is as follows: a component can be discontinued only if there are no existing cars with that component today.

Initially, I implemented this in a service class.

public boolean canBeDiscontinued(CarComponentType carComponentType) {
    List<Car> cars = carRepository.getCarsWithComponent(carComponentType);
    return cars.isEmpty();
}

This works - but this logic is used from several other places in the code. It might grow, and it looks like something that could fit inside the CarComponentType class instead:

public boolean canBeDiscontinued() {
    List<Car> cars = carRepository.getCarsWithComponent(this);
    return cars.isEmpty();   
}

However, I can't put it there, since it needs to access the repository (and as I understand it is a very serious antipattern for entities to be aware of the data access layer). When loading a component type I can't load all cars of that type since that could be thousands of objects. We are not using any ORM, so making a lazy loaded collection for not only be bulky but also very error-prone.

Is it more appropriate to actually have this method in a service class as I first did? Is it not important? Is there another alternative? Should I start refactoring from another starting point?

There is a similar question here. But my question relates to Java, so I don't think that solution is applicable in my case. Also, sorry in advance for using cars and components as my domain model. :)

Answer 1

This sounds like a good candidate for the Specification pattern

Answer 2

Frederik Gheysels answer is good, although perhaps a bit short. To elaborate: In your case, you could start out by defininig an interface for your specification (excuse my C# syntax):

public interface ICarSpecification
{
    bool IsSatisfiedBy(CarComponentType carComponentType);
}

You can then create an implmenetation of ICarSpecification that uses your Repository. Something like this:

public class CanDiscontinueSpecification : ICarSpecification
{
    private readonly CarRepository carRepository;

    public CanDiscontinueSpecification(CarRepository carRepository)
    {
         this.carRepository = carRepository;
    }

    public bool IsSatisfiedBy(CarComponentType carComponentType)
    {
        return this.carRepository.GetCarsWithComponent(carComponentType).Any();
    }
}

You could stop right there, but the thing I don't particularly like about the Specification pattern is that it is not very discoverable. One possible solution would be to inject the Specification into the CarComponentType itself:

public class CarComponentType
{
    private readonly ICarSpecification discontinueSpec;

    public CarComponentType(ICarSpecification discontinueSpec)
    {
        this.discontinueSpec = discontinueSpec;
    }

    public bool CanBeDiscontinued()
    {
        return this.discontinueSpec.IsSatisfiedBy(this);
    }
}

Alternatively, if you don't like to carry around the Specification in every instance of the class, you could use Method Injection instead of Constructor Injection:

public bool CanBeDiscontinued(ICarSpecification spec)
{
    return spec.IsSatisfiedBy(this);
}

Such a method doesn't really add any value in terms of implementation, but is more discoverable.

Answer 3

I don't think it's an understatement to say I hate anemic domain models as much as the next man.

However given the system you are working on has already has established the anemic domain model/service (anti) pattern I think introducing additional patterns could be to the determent of the system when done in isolated cases as I believe you have here. Such a decision should be made by the team and should have clear benefits.

Also, IMHO, I think your original code snippets are simpler than the solutions proposed in other answers (no offense Mark and Frederik, just an observation :-) and that the more complicated solution doesn't really bring any benefits - I mean, the functionality is the same in both cases but the later uses more moving parts.

In terms of how to proceed, with a single example it's hard to say. Introducing an ORM (which you mentioned isn't currently used) could be one way to go as it should reduce the code in you service classes and would have clear benefits so I'd be tempted to start there, then once that's in place review the situation.

Answer 4

I don't think the "can this be discontinued" belongs in any of those classes. Who is responsible for determining if a part can be discontinued? Not the Car or the Car component. I think you were on the right track with your initial method implemented in a service. Perhaps you need an CarInventoryManagementService that's responsible for answering questions about car inventory items like:

carsUsingComponent( CarComponent comp )
canComponentBeDiscontinued( CarComponent comp )
etc

If you have multiple places in the code that need to ask inventory related questions, such as your "canBeDiscontinued" then it might make sense to create a service just with that responsibility.