Massive controller constructor argument list when

Question

I am working on ASP.NET MVC3 solution that uses dependency injection with autofac. Our controllers are being created by autofac and properly and all required objects are being properly passed in. Those objects usually include services, repositories, and mappers converting domain object to MVC (view) models. So the controller constructor looks somewhat like:

public abcController(
        ILogger logger,
        IabcRepository abcRepository,
        IabcService abcService,
        IMapper<AbcDomain, AbcViewModel> abcMapper,
        ...
        )

Unfortunately, in time, those constructor parameter lists tend to grow quite quickly. Some of our controllers expect now 60 or more parameters.

Did we create some anti-pattern here?

EDIT

I should have mentioned that we try to follow thin contoller pattern. Also most of those parameters tend to be mappers - around 66%. The control methods are usually very simple, and follow either this pattern:

• Based on parameters call appropriate service or repository
• Use mapper to convert result to appropriate view model
• Pass view model to view

Or this pattern:

• Receive model from post action
• Use mapper to convert it to appropiate domain object
• Invoke appropriate service or repository with domain object

Answer 1

If a lot of this is down to creating viewmodels this question and answer may help.

MVC - Controller with multiple select lists

Also I'd look at MVC 4 in Action from Manning. It covers creating an ActionResult that automates the mapping.

In my applications most of my controller actions are one line. Either pulling an entity and passing it to an Automapping and enriching viewresult or taking in a command and passing it to an action result that processes it

This blog post from Jimmy covers some of the POST side http://lostechies.com/jimmybogard/2011/06/22/cleaning-up-posts-in-asp-net-mvc/

Basically I get a domain object (from repo or other method) and return it with an automapped view result which maps to appropriate VM.

return AutoMappedView<ExaminationCreateModel>(new Examination ( _assetRepository.Find(assetId)));

The mapper ViewResult then passes it to an enricher (if one is found implementing IModelEnricher. See other stack question.

On return it is posted back as a command, command is then handled a bit like Bogard post.

    public virtual ActionResult Create(AddAssetExaminationCommand addAssetExaminationCommand, ICommandHandler<AddAssetExaminationCommand> addExaminationHandler) 
    {
        return ProcessForm(
            addAssetExaminationCommand,
            addExaminationHandler,
            RedirectToAction(MVC.OnboardAsset.Examinations.Create()),
            RedirectToAction(MVC.OnboardAsset.Examinations.Index(addAssetExaminationCommand.AssetId)));
    }

If it fails validations then it redirects to GET and the Modelstate is merged (PRG pattern using something like this) so errors persist. If it is valid command handler deals with it and we redirect to a success page

Answer 2

(Disclaimer: This answer was in regards to the size of the argument list. It does not reduce the dependencies within the controller)

In this case, you would inject a Factory.

For example:

interface IABCFactory {
    ILogger CreateLogger();
    IABCRepository CreateRepo();
    // .. etc
}

Your constructor then becomes:

private ILogger _logger;

public abcController(IABCFactory factory) {
    _logger = factory.CreateLogger();
    // .. etc
}

Note, you can inject into public properties.. but it is up to you whether you want to expose that to the outside world. If you don't want to break encapsulation, then you would go with a factory.

Answer 3

I can't really speak to how you should rearchitect your controller, though I agree with most of the other answers - 60 incoming parameters is a lot.

Something that might help reduce the number of parameters but not the number of dependencies is the Aggregate Service support that Autofac has.

Instead of taking 60 parameters directly, you can take a single aggregate parameter that has 60 properties on it.

You create an interface (just the interface, you don't actually have to implement it) with the dependencies:

public interface IMyAggregateService
{
  IFirstService FirstService { get; }
  ISecondService SecondService { get; }
  IThirdService ThirdService { get; }
  IFourthService FourthService { get; }
}

Then modify your controller to take that aggregate interface:

public class SomeController
{
  private readonly IMyAggregateService _aggregateService;

  public SomeController(
    IMyAggregateService aggregateService)
  {
    _aggregateService = aggregateService;
  }
}

You can register your aggregate service interface, your dependencies, and your controller and when you resolve the controller, the aggregate service interface will automatically be implemented and resolved for you.

var builder = new ContainerBuilder();
builder.RegisterAggregateService<IMyAggregateService>();
builder.Register(/*...*/).As<IFirstService>();
builder.Register(/*...*/).As<ISecondService>();
builder.Register(/*...*/).As<IThirdService>();
builder.Register(/*...*/).As<IFourthService>();
builder.RegisterType<SomeController>();
var container = builder.Build();

Again, it doesn't speak to the larger issue of requiring that many dependencies, but if you are just looking to simplify your constructor and the number of properties on the controller so it's more manageable, this is one strategy Autofac offers to help in that.

Check out the wiki page for more details.

Answer 4

60 or more parameters is a lot.

In your question you stated "..Those objects usually include services, repositories, and mappers converting domain object to MVC (view) models..."

You've got a Fat Controller (not the Thomas The Task Engine kind) but a controller that is doing too much.

The balance I look for is Fat Model skinny controller. Ian Cooper talks about it well in this blog post

You can also look at things such as which parameters are actually cross cuttings concerns.

For instance Mapping and Logging in my mind are cross cutting concerns so you could potentially use Action Filters to clean up your controllers.