I am using MVC 3. Suppose I have a Base Controller and several derived controllers. I am using IoC and my base controller constructor looks like this:
private readonly ICacheProvider _cacheProvider;
private readonly ILoggerProvider _loggerProvider
private readonly IAuditProvider _auditProvider
public abstract class MyControllerBase : Controller
{
protected MyControllerBase(ICacheProvider cacheProvider, ILoggerProvider loggerProvider, IAuditProvider auditProvider, ...)
{
_cacheProvider = cacheProvider;
_loggerProvider = loggerProvider;
_auditProvider = auditProvider;
...
}
}
So far so good? Maybe. However, each of my derived controllers needs to define a constructor that matches the base class constructor signature, for example:
public class MyDerivedController1 : MyControllerBase
{
public MyDerivedController1(ICacheProvider cacheProvider, ILoggerProvider loggerProvider, IAuditProvider auditProvider, ...)
: base(cacheProvider, loggerProvider, auditProvider, ...)
{ }
}
And that is my problem, because I have to maintain the "verbose" constructor in all my derived controllers. If I need to add a new provider I have to refactor all my derived controllers.
I thought I would create a ServiceProvider (or Service Locator ?) class (and IServiceProvider interface), which would have a constructor and all providers as parameters (where IoC would do its job) and expose them as properties. Then my base constructor and derived constructors would only have the IServiceProvider as a parameter.
However I am concerned that this approach will have some negative impacts, for example: 1- Hidden implementation: I don't know which provider I use or need unless I check the implementation. 2- Hard to test: when the constructor contains parameters I can easily test it and I know what to expect (auto-documented).
Does anyone have any suggestion or comments?
After some research I was unable to find a good solution. As mentioned by Daniel Hilgarth the controller has too many dependencies which violates SRP. I agree. Given that this is an existing application I cannot refactor and re-design the whole thing. For example, I would like to move dependencies such as ICacheProvider, ILoggerProvider and IAuditProvider to another layer, which would be responsible for retrieving the data from the repository.
I don't want to start a new discussion here, but I don't like the idea of referencing Entity Framework in my MVC web project. I would prefer to create a Data Access layer and remove completely the references to EF. So, back to my problem, I have a Base Controller with X dependencies and all derived controllers need a constructor with all those references (as explained in my question above).
I've considered a few options:
1) Using a Service Locator. After reading quite a few posts, I've decided to ignore this option.
IoC.Resolve vs Constructor Injection
http://blog.ploeh.dk/2010/02/03/ServiceLocatorisanAnti-Pattern/
2) Using Aggregate Services. This is a good idea, but I couldn't group my dependencies and I decided to ignore this one too.
How to avoid Dependency Injection constructor madness?
http://blog.ploeh.dk/2010/02/02/RefactoringtoAggregateServices/
3) Creating a new Provider which exposes all the other dependencies as properties. I am not completely satisfied with this option because it somehow hides the implementation details. But after all considerations I have decided to implement it. Find below the new interface and class created:
public interface IMyControllerProvider
{
ICacheProvider CacheProvider { get; }
ILoggerProvider LoggerProvider { get; }
IAuditProvider AuditProvider { get; }
...
}
public class MyControllerProvider : IMyControllerProvider
{
private readonly ICacheProvider _cacheProvider;
private readonly ILoggerProvider _loggerProvider;
private readonly IAuditProvider _auditProvider;
.....
public MyControllerProvider(ICacheProvider cacheProvider, ILoggerProvider loggerProvider, IAuditProvider auditProvider, ...)
{
_cacheProvider = cacheProvider;
_loggerProvider = loggerProvider;
_auditProvider = auditProvider;
...
}
public ICacheProvider CacheProvider { get { return _context; } }
public ILoggerProvider LoggerProvider { get { return _context; } }
public IAuditProvider AuditProvider { get { return _context; } }
}
After that I refactored my base controller and all derived controllers to use IMyControllerProvider instead. It works fine because:
1- The derived controllers have now a constructor with a single dependency.
2- Although the new dependency IMyControllerProvider "hides" the real dependencies, when testing the controller it is still easy to understand that there are other dependencies (the constructor documents that).
private readonly ICacheProvider _cacheProvider;
private readonly ILoggerProvider _loggerProvider
private readonly IAuditProvider _auditProvider
public abstract class MyControllerBase : Controller
{
protected MyControllerBase(IMyControllerProvider myControllerProvider)
{
_cacheProvider = myControllerProvider.CacheProvider;
_loggerProvider = myControllerProvider.LoggerProvider;
_auditProvider = myControllerProvider.AuditProvider;
}
}
public class MyDerivedController1 : MyControllerBase
{
public MyDerivedController1(IMyControllerProvider myControllerProvider)
: base(myControllerProvider)
{ }
}
Not the best solution, but the only one I could come up with given the restriction of refactoring the whole application.