Question

BACKGROUND

For my own understanding, I'm trying to create a layered architecture for a simple scenario.  I have a simple domain class, Car.  

public class Car {

  public string Make;
  public string Model;
  public int Year;

  public Accelerate() { }
  public Decelerate() { }
}

I want to create a new instance of Car populating its values from a database.  This is where I get stuck.

I think there should be another class in a Persistence layer (called CarRepository) that will do the dirty work of retrieving those values from a database based on a CarID.

I hope to inject this CarRepository dependency into the Car domain class at runtime to allow for different implementations (like fakes for unit testing). 

So, I consider adding this to the Car class:

public class Car() {

  private ICarRespository _carRepository;

  public Car(ICarRepository carRepository) {
    _carRespository = carRepository;
  }

  public void Find(int carId) {
    var car = _carRepository.FindById(carId);
    this.Make = car.Make;
    this.Model = car.Model;
    // etc.
  }

  // other properties and methods here
}

And I would define the ICarRepository as:

public interface ICarRepository {
  DtoCar FindById(int carId);
}

public class DtoCar {
  public string Make;
  public string Model;
  public int Year;
}

Let's assume I'm using "poor man's dependency injection" and allowing the aggregate root to pass the right concrete CarRespository based on the project type (production code vs. unit test).  Obviously I've chosen to use Constructor Injection here.

QUESTIONS

(1) This solution doesn't feel right.  When I create a new Car, I feel it should be initialized at that point with all its properties populated.  However, it can only really be a valid car AFTER I call the Find() method.  What happens if I don't pass a valid CarID to Find?  Then the Car object will never be a valid object!  Should this not bother me?  Is there a better way?

(2) I'm using Constructor Injection to pass the repository dependency in … could I also pass the CarID to find here in the constructor as well … I could then eliminate the Find() method?  So:

public Car(ICarRepository carRepository, int carId) {
  _carRespository = carRepository;
  var car = _carRepository.FindById(carId);

  this.Make = car.Make;
  this.Model = car.Model;
  // etc.
}

... I've never seen it done like this so I'm hesitant to use it. Is it ok to do it this way?

(3) Is there some creational pattern I'm missing here?  This is a simple scenario … I don't believe things like Factory, Singleton or Prototype apply here (I could be mistaken).  Those feel heavy and specialized.  This scenario -- a simple domain object from a database -- seems so basic that there must be some guidance.

Was it helpful?

Solution

Your problem is that you are not following the Single Responsibility Principle. You should offload the creation of the car to a service that specializes in this.

class CarService
{
    CarService(ICarRepository carRepo)
    { 
        _carRepo = carRepo;
    }

    Car GetById(int id)
    {
        var carDTO = _carRepo.GetById(id);       
        var car = Mapper.Map<CarDTO, Car>(carDTO);
        return car;
    }
}

*Here is the link to AutoMapper

**You don't even need the CarDTO, however it would add a layer of abstraction if needed.

This will act like a factory, but the implementation makes more sense and is not heavy as you feared. The code will read more like this:

var car = carService.GetById(id);

This should solve all of your questions, I believe.

OTHER TIPS

The design of Car class violates Single responsibility principle: its a domain entity and it contains data persistence logic.

So, let's Car be just a domain entity (ID property has been introduced):

public class Car
{
    public Car(int id, string make, string model, int year)
    {
        ID = id;
        Make = make;
        Model = model;
        Year = year;
    }

    public int ID { get; private set; }
    public string Make { get; private set; }
    public string Model { get; private set; }
    public int Year { get; private set; }

    public void Accelerate() { }
    public void Decelerate() { }
}

The repository interface must not return data transfer objects:

public interface ICarRepository
{
    Car FindById(int carId); // No DTO here - DTO is the detail of implementation
}

Then, just design the interface for the Car service. Service layer exposes business logic. In this simple case the service layer adds no value:

public interface ICarService
{
    Car FindById(int carId);
}

public class CarService : ICarService
{
    private readonly ICarRepository _repository;

    public CarService(ICarRepository repository)
    {
        _repository = repository;
    }

    #region Implementation of ICarService

    public Car FindById(int carId)
    {
        var car = _repository.FindById(carId);
        // The additional business logic (if required) could be added here.
        return car;
    }

    #endregion
}

The actual implementation of the repository will create the Car using its constructor.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top