Elegantly reducing the number of dependencies in ASP.NET MVC controllers

Question

We are developing what is becoming a sizable ASP.NET MVC project and a code smell is starting to raise its head.

Every controller has 5 or more dependencies, some of these dependencies are only used for 1 of the action methods on the controller but obviously are created for every instance of the controller.

I'm struggling to think of a good way to reduce the number of objects that are created needlessly for 90% of calls.

Here are a few ideas I'm toying around with:

1. Splitting the controllers down into smaller, more targeted ones.
   • Currently we have roughly a controller per domain entity, this has led to nice looking URLs which we would like to emulate, meaning we would end up with a much more complicated routing scheme.
2. Passing in an interface wrapping the IoC container.
   • This would mean the objects would only be created when they were explicitly required. However, this just seems like putting lipstick on a pig.
3. Extending the framework in some way to achieve some crazy combination of the two.

I feel that others must have come across this same problem; so how did you solve this or did you just live with it because it isn't really that big a problem in your eyes?

Answer 1

There is the concept of "service locator" that has been added to works like Prism. It has the advantage of reducing that overhead.

But, as you say, it's just hiding things under the carpet. The dependencies do not go away, and you just made them less visible, which goes against one of the goals of using DI (clearly stating what you depend on), so I'd be careful not to overuse it.

Maybe you'd be better served by delegating some of the work. If there is some way you were intending to re-partition your controller, you might just want to create that class and make your controller obtain an instance of it through DI. It will not reduce the creation cost, since the dependencies would still be resolved at creation time, but at least you'd isolate those dependencies by functionality and keep your routing scheme simple.

Answer 2

I've been pondering solutions to this very problem, and this is what I've come up with:

Inject your dependencies into your controller actions directly, instead of into the controller constructor. This way you are only injecting what you need to.

I've literally just whipped this up, so its slightly naive and not tested in anger, but I intend to implement this asap to try it out. Suggestions welcome!

Its of course StructureMap specific, but you could easily use a different container.

in global.asax:

protected void Application_Start()
{
    ControllerBuilder.Current.SetControllerFactory(
            new StructureMapControllerFactory());
}

here is structuremapcontrollerfactory:

public class StructureMapControllerFactory : DefaultControllerFactory
{
    protected override IController GetControllerInstance(Type controllerType)
    {
        try
        {
            var controller = 
                    ObjectFactory.GetInstance(controllerType) as Controller;
            controller.ActionInvoker = 
                    new StructureMapControllerActionInvoker();
            return controller;
        }
        catch (StructureMapException)
        {
            System.Diagnostics.Debug.WriteLine(ObjectFactory.WhatDoIHave());
            throw;

        }
    }
}

and structuremapcontrolleractioninvoker (could do with being a bit more intelligent)

public class StructureMapControllerActionInvoker : ControllerActionInvoker
{
    protected override object GetParameterValue(
            ControllerContext controllerContext, 
            ParameterDescriptor parameterDescriptor)
    {
        object parameterValue;
        try
        {
            parameterValue = base.GetParameterValue(
                    controllerContext, parameterDescriptor);
        }
        catch (Exception e)
        {
            parameterValue = 
                    ObjectFactory.TryGetInstance(
                            parameterDescriptor.ParameterType);
            if (parameterValue == null)
                throw e;
        }
        return parameterValue;
    }
}

Answer 3

I would consider separately the problem of dependencies and creation of dependent objects. The dependency is simply the fact that the controller source code references a certain type. This has a cost in code complexity, but no runtime cost to speak of. The instantiation of the object, on the other hand, has a runtime cost.

The only way to reduce the number of code dependencies is to break up the controllers. Anything else is just making the dependencies a bit prettier, as you say. But making the dependencies (as opposed to instantiation of the dependent objects, which I'll cover in a second) prettier may well be enough of a solution that you don't need to break up the controllers. So IoC is a decent solution for this, I think.

Re: creating the objects, you write, "...some of these dependencies are only used for 1 of the action methods on the controller but obviously are created for every instance of the controller." This strikes me as the real problem, rather than the dependency, per se. Because it's only going to get worse as your project expands. You can fix this problem by changing the instantiation of the objects so that it does not happen until they are needed. One way would be to use properties with lazy instantiation. Another way would be to use arguments to your action methods with model binders which instantiate the objects you need. Yet another way would be to write functions which return the instances you need. It's hard to say which way is best without knowing the purpose of the objects you're using.

Answer 4

Your controllers may be becoming too "Fat". I suggest creating an application tier which sit beneath your controllers. The application tier can encapsulate a lot of the orchestration going on inside your controller actions and is much more testable. It will also help you organize your code without the constraints of designated action methods.

Using ServiceLocation will also help (and yes, I'm essentially reiterating Denis Troller's answer- which probably isn't good but I did vote his answer up).