Search code examples
c#genericsdomain-driven-designinversion-of-controlmvp

Ioc with service class calling generic method


I having a bit of trouble getting my head around Ioc and generics, and particularly how my company has set up the layers in relation to this. We're working under an MVP architecture.

I have a Car class:

class Car : ICar
{
   IList<IWheel> wheels{get;set;}
   IEngine engine{get;set;}
   string registrationplate {get;set;}

   public Car(){}
}

and I want to be able to get a newly created ICar, and I also want to be able to find an ICar by Id. Problem is I'm not sure why some of the design choices have been made in the project I'm working on. The structure for other services already created is as follows:

I have a Car and WHeel Service :

class WheelService : BaseService
{
    IWheel wheel;

    public IWheel Create()
    {
        return new wheel()
    }

 }
class CarService : BaseService
{
   WheelService wheelservice;

   public ICar CarCreate()
   {
     wheelservice = new Wheelservice()
     car = new Car();
     IWheel wheel = wheelservice.Create();
     car.wheels.add(wheel);
     return car;
   }

   public ICar Find(int id)
   {
        return (Car)base.Find<Car>(id);
   }

}
  1. Firstly, I'm finding the 'have a service for each entity' odd. I wouldn't have thought that a weak entity would have a service. My thoughts would be that the CarService create method would act like a factory method, without having to call a wheel service.
    Also, the wheel service create method is actually used in the presenter code to return an IWheel all the way down to the UI, so that values can be set and passed back up. Again this, seems odd to me. Means the presenter can request the full ICar object from the UI.

  2. Is the dependency in the Service create method normal? I would have thought that this would be brought in through IoC. But, if the ICar Create method was to handle all creation (including wheel, etc), then presumably the container would contain lots of interfaces in relation to this particular service?

  3. If I were to bring in the interfaces, I would need to adjust the CarService.Find method, which is currently using concrete classes. It uses the BaseService, and it is solely this layer which interacts with the container to get the correct repository:

class BaseService
{
private object myRepository;

 protected T GetRepository<T>(Type serviceType)
    {

        if (myRepository == null)
        {
            myRepository = (T)IoCRepositoryFactory.GetRepositoryInstance(typeof(T), serviceType);
        }
        return (T)myRepository;
     }


protected virtual IGenericRepository Repository
    {
        get
        {
            return GetRepository<IGenericRepository>(this.GetType());
        }
    }

protected virtual T Find<T>(Object id) where T : class
    {
        return (T)Repository.GetByID<T>(id);
    }

}

I'm unsure how to call this find method if I'm only using interfaces, since the current service definition is using concrete classes.

Apologies about this being a long winded post. I've been looking over this for three days, but I need to know what others think of the current set up, and whether I should be using IOC for domain objects in service layer, or whether I should follow the current set up. It just all feels a bit coupled for me at the moment.


Solution

  • There's a lot you're misunderstanding I think.

    I'll start with the service structure. You don't have to have each service only handling 1 type of entity, but there's nothing wrong with that as long as it doesn't get in the way of efficiency. Your example is likely unreasonably simplistic, so you probably could have CarService handle Wheel management without any future maintenance issues, but it's common to divide services up by entity and although it often requires some exceptions it usually works fine.

    The IoC code you have written is all sorts of wrong though. The goal of IoC is to keep all the code that will be doing your dependency management in a single place that's sort of out of the way. Your code has CarService instantiating WheelService explicitly, while that should be handled by your IoC container. Also, having a GetRepository method is very awkward, and also should be handled automatically by your IoC container.

    One way you could setup your services and repositories would be like this (if you're using constructor injection). This is just an example (and a verbose one at that), don't go copying this structure into your own code without fully understanding it.

    public interface IRepository {
        //Some interfaces
        //This method could only really be implemented if you are using an ORMapper like NHibernate
        public T FindById<T>(Object id) { }
    }
    
    public class BaseRepository : IRepository {
        //Some base crud methods and stuff. You can implement this if you're using an ORMapper
        public T FindById<T>(Object id) 
        { 
            return CurrentSession.FindById<T>(id);
        }
    }
    
    public class WheelRepository : BaseRepository {
        //Wheel crud
    
        //If you don't have an ORMapper because you're a masochist you can implement this here
        public Wheel FindById(Object id) { }
    }
    
    public class CarRepository : BaseRepository {
        //Car crud
    
        //If you don't have an ORMapper because you're a masochist you can implement this here
        public Car FindById(Object id) { }
    }
    
    public class BaseService {
        protected BaseRepository _baseRepository;
    
        //This constructor is automatically called by your IoC container when you want a BaseService
        public BaseService(BaseRepository repository)
        {
            _baseRepository = repository;
        }
    
        //More methods
    }
    
    public class WheelService : BaseService
    {
        protected WheelRepository _wheelRepository;
    
        public WheelService(WheelRepository wheelRepo) : base(wheelRepo)
    }
    
    public class CarService : BaseService
    {
        protected WheelService _wheelService;
        protected CarRepository _carRepository;
    
        public CarService(WheelService wheelService, CarRepository carRepository)
        {
            _wheelService = wheelService;
            _carRepository = carRepository;
        }
    }
    

    Since you're using MVP, I assume some kind of WPF/Winforms app, although I suppose you could do a web app as well if you really wanted to make it fit in asp.net. In either case, they both have a single Factory you can configure for creating your presenter classes. In that factory, you'd have it do all the injection there, out of the way in a spot you never have to worry about or even look at after setting it up. That code would just call this:

    //Override the default factory method (just made this up)
    public override Presenter GetPresenter(Type presenterType)
    {
        return ComponentFactory.GetInstance(presenterType);
    }
    

    Then if you had a presenter that depended on a service, it would automatically be there, and everything that service needs would be there too. Notice how there's no ugly service constructors or factory calls anywhere else. All the dependencies are automagically setup by the container.

    I don't know why the code at your company has those awful GetRepository methods. That's an instance of an anti-pattern, because you're replacing what would normally be a constructor call like new Repository() with a GetRepository call instead that's only marginally more maintainable.

    You should try playing around with some well-known containers. Structuremap is a pretty good one.