Controller injects Service class dependencies OR leave it to Service to resolve its own dependencies ? what is the preferred way

Question

I have the following classes, with latter service class is coupled to a specific container, but if I use former why should controller class inject service class dependencies instead of just using the service class and leave it to service class to resolve the dependencies ?

public class ProfileController {

    public ProfileController(IProfileService profileService, IProfileRepository  profileRepository, IUnitOfWork unitOfWork, IDatabaseFactory databaseFactory) {
    profileService.Get();
    }
}

public class ProfileService:IProfileService {

    public ProfileService(IProfileRepository  profileRepository, IUnitOfWork unitOfWork, IDatabaseFactory databaseFactory) {
    profileService.Get();
    }  
}

or

public class ProfileController {
    public ProfileController(IProfileService profileService) {
                            profileService.Get();
    }
}

public class ProfileService:IProfileService {

    public ProfileService() { 
        var dbfactory=container.Resolve<IDatabaseFactory>();
        var unitofwork=container.Resolve<IUnitofWork>();
        var rep=container.Resolve<IProfileRepository>();
        rep.Get();
    }  
}

Answer 1

I feel both examples are off track.

Proposed Solution:

public class ProfileService:IProfileService {

  public ProfileService(IProfileRepository  profileRepository, IUnitOfWork unitOfWork, IDatabaseFactory databaseFactory) {

  }  
}

with the following use of that service in the caller in the following way.

public class ProfileController {
  [ConstructorInjection]
  public ProfileController(IProfileService profileService) {

  }
}

Or

public class ProfileController {
  public ProfileController(IProfileService profileService) {

  }
}

or

public class ProfileController {

  public ProfileController() {
       this.service = resolveFromContainer.Get<IProfileService>();
  }
}

or

public class ProfileController {

  public ProfileController() {
       this.service = new ServiceFactory.get<IProfileService>()
  }
}

(1). In either case above, I don't see separation of concerns as an issue in either case. The controller is not doing the repositories job. It is only providing the respository to use.

As far as exposing modules go. For shared assemblies I agree 100% we should not force the callers to use the di container.

However we can build shared code and let the consuming application choose how to inject the shared module dependency.

(2).Marc mentions this in his answer "Semantic clarity: Your constructor parameters should answer the following question: What do I need to create an instance of that class. In your case, this is stuff: IDatabaseFactory, etc. and not nothing (())."

I agree 100%, however if you see the controller constructor. It not only has service but has a lot of other stuff as well. Do I need everything else for my controller to work ? This is an application specific question and answer can't be genericized. If you only need the service to work then pass that in. Do not pass service dependencies as that doesnt define the controller. it defines the service.

Passing a Repository is different from passing developer keys to a library. Although you are allowing the flexibility to provide the repository, you are also specifying the destination. This is useful in some cases .. think of library that stores logging information ( we are specifying where to store) also useful in RequestProcessor patterns where the code is abstracted out.

Q:If you expect your service to save and return then Is it right to pass the repository, dbFactory to it ?

In the new solution above. The profile service code doesn't change. the profile service may require DB, Repository But does the controller needs all of repository, factory, service, to work ? This is an application specific

*Q: Do the parameters passed in needed to create the controller or are some of them being indirectly used to create other dependencies underneath? *

(3) "container.Resolve(...) should not occur in your modules."

*Q:What are modules you refer to ? Are you saying that we should use the DI container but somehow never choose to use DI attributes/resolve methods/dependency configuration at any piece of code? *

(I can see authors of di container cringe on that thought)

Q:Can you answer, Where is it okay to use resolve method?

I agree 100% that shared code should not use resolve methods. However the controller can as is application specific code and does need the container to inject/assemble its dependencies.

The DI container is a core framework that is a first class citizen of your application design. If we haven't picked one DI container to build against, have we really embraced DI?

If DI container is a first class citizen like .NET libraries then you would never 'plan' to change your container without additional work. Shooting for a code that easily allows you to switch DI containers is too much work and is best left for Microsoft/community to solve.

Summary: I agree with most of Marc's thoughts on the answers *however either solution proposed answer doesnt answer all the question he has raised correctly. *

References:

1. http://martinfowler.com/articles/injection.html
2. http://msdn.microsoft.com/en-us/magazine/dd882510.aspx

Minor gripes: Small comment on either solution above on the use of Unit of Work/ Respository

Quoted code from links above. public void RunCommands(Invoice invoice) { IUnitOfWork unitOfWork = _unitOfWorkFactory.StartNew();

The UnitOfWorkfactory should manage the database factory and that should not be passed in to the Service.

IMHO

We are doing the same handholding of dependencies that doesnt feel right to me. The same applies to the respository factory as well.

The factory should set the context like db context etc. that responsibility should not be even in the service.

This may be better interface public ProfileService(IProfileRepository profileRepository, IUnitOfWork unitOfWork)

Answer 2

The service class should not resolve it's dependencies. So you should go for option 1. The instances should be completely agnostic of the DI container.

Why?

Seperation of concerns: The big idea of dependency injection is to consider the instantiation of objects to be a distinct concerned, which is taken care of in a single place: The container. It's the container's job to create instances and to set references between them. You undermine the concept when you let the instances itself use the container.

Reusability of your module classes: When you have a class which 'uses' the DI container, you can't use this class in a different architecture anymore, including a different DI framework, without changing it. Although you might not want to, you limit yourself without good reason.

Semantic clarity: Your constructor parameters should answer the following question: What do I need to create an instance of that class. In your case, this is stuff: IDatabaseFactory, etc. and not nothing (()).

Testability: You don't want your classes to require the whole DI infrastructure to work. Perhaps for testing reason a mock up should do, what at runtime is done by your DI container. This is linked to 'Reusability'.

Fail fast: Assumed the dependencies are not resolvable: It is nice to have the exception thrown when building the container and not when instantiating the class.

Summary:

container.Resolve(...)

should not occur in your modules.