DependencyInjection – Constructor over-injection smell vs service locator – where is the proper approach

code smelldependency-injectioninversion-of-controlioc-containersservice-locator

I am trying to improve my MVC projects and I read a few articles about DI, IoC containers, Constructor Injection and Service locators. I wanted to go with Ninject to help with the dependencies, but got puzzled with some of the stuff I read.

The most controversial would be the widely discussed one here http://jeffreypalermo.com/blog/constructor-over-injection-anti-pattern (both parts).

And then some responses to it, like Mark Seemann's response, and a follow up

Jeffrey expreses my fears and addresses them in an incorrect manner – however, I am not sure what to do in my case.

For example, lets look at an MVC controller. It serves as a communication layer between an MCV App and a WebAPI. I'm a bit concenrned about the number of dependencies I would have when doing Constructor Injection.

public class OcrController : ConnectorControllerBase, IOcrController
{
    private readonly ILogger logger;
    private readonly IConnectorConfigurator config;
    private readonly HttpClient client;
    private readonly IOcrConnector connector;

    public OcrController(
            ILogger logger, 
            IConnectorConfigurator config, 
            HttpClient client, 
            IOcrConnector connector) 
    {
        this.logger = logger;
        this.config = config;
        this.client = client;
        this.connector = connector;
    }
}

While the IOcrConnector and HttpClient are clear depenendencies that I have no issues with, I am concerned about the other two. Depending on an action , it require an IConfiguration to read something from the config and similarily a Logger might be needed in some cases.

Same would be true for many of my classes – one or two actual dependencies, and the logger and config and maybe something else in the future.

At the moment I am using a simple and poor ServiceLocator, where instead of passing the ILogger and IConfig to constructor, I create them when needed:

protected ILogger Logger => this.logger 
    ?? (this.logger = ComponentFactory.GetLogger(this.GetType()));

protected IConnectorConfigurator Config => this.config 
    ?? (this.config = ComponentFactory.GetConfig());

The 'Factory' is very simple, because it simply returns my ILogger wrapper like below: 

public static ILogger GetLogger(Type type)
{
    return new Log4NetLogger(type);
}

This factory lives in a separate assembly, so it seems to me it decouples the concrete ILogger from the actual projects sufficiently…?

I do not intend to be exchanging the ILogger or IConfig (which is now just a wrapper for built-in ConfigurationManager) at runtime or whatever, but when/if the need arises, I would just change the type returned from the GetLogger(Type type); method.

So, please let me know if that kind of mix of proper DI and ServiceLocator makes sense?
Other alternative I think of would maybe be passing an IComponentFactory into the constructor (instead of having a globally accessed static?) But that is still a service locator, only placed elsewhere…

Please let me know what you think

Best Answer

In your linked example the blogger is arguing that the code should be refactored because the dependency 'is passed down the chain' rather than being used directly.

Its a 'Hobo parameter' code smell rather than anything to do with DI. He could fix it by injecting into Order instead of OrderProcessor

In your example your controller will presumably directly used ILogger? So its not the same issue.

Moveover MVC controllers will always require many service classes to be injected as they must necessarily be designed in a procedural way.

Consider that a Method call on your controller which takes an Order parameter must construct that object from the pure value data in the request via binding.

Because the Order input parameter is created by this pipeline you must inject the dependency into the controller or the binder.

Either way the pipeline must act as the dependency setup for the call in exactly the way the blogger objects to, because the Order object must be created from the incoming data rather than be passed in by reference

Related Topic