Refactoring Bloated Controllers with IServiceProvider
I have come into an ASP.NET MVC project that has a HomeController that takes at least 10 service arguments in its constructor. My first line of attack is to factor out the Action methods into separate controllers that only need a subset of the injected services to work properly. So, first and foremost I recognize I need more and smaller controllers with cleaner separation of concerns.
Of course, things are never that easy and for reasons not worth mentioning that restructuring is likely weeks, if not months, away. That said, I can’t sleep at night with those controllers in their current form.
HomeController with many services injected.
public class HomeController : Controller
{
private readonly ILogger<HomeController> _logger;
private readonly IArrayReversalService _arrayReversalService;
// more fields for injected services
public HomeController(ILogger<HomeController> _logger, IArrayReversalService _arrayReversalService, ... more services)
{
_arrayReversalService = serviceProvider.GetService<IArrayReversalService>();
_logger = serviceProvider.GetService<ILogger<HomeController>>();
// More services assigned
}
}
What I am about to do with IServiceProvider directly is an example of the Service Locator Pattern. It’s usually a bad idea, as it hides a class’s dependencies. But in my case, I will use it to make my refactoring go more smoothly
I am looking at IServiceProvider to at least localize dependencies to the local method that needs them. So far, there’s no need to do anything different in Program.cs to register the services.
// dependencies are injected like any others in Programs.cs
builder.Services.AddTransient<IArrayReversalService, ArrayReversalService>();
With that, I can inject just the IServiceProvider as a single dependency that allows access to the registered service objects. Since I am likely going to use _logger
in all my controller action methods, I’ll just pull that service out in the constructor for future use.
private readonly ILogger<HomeController>? _logger;
private readonly IServiceProvider _serviceProvider;
public HomeController(IServiceProvider serviceProvider)
{
_serviceProvider = serviceProvider ?? throw new ArgumentNullException(nameof(serviceProvider));
_logger = _serviceProvider.GetService<ILogger<HomeController>>() ?? throw new InvalidOperationException();
}
That’s one way I can use the service in IServiceProvider, just store it as a field during construction as you likely do today. One of my goals in moving to this model, as you may recall, is to factor out Action methods into logical controllers so I don’t have one large master controller. To get started, I’ll pull out only those services we need in the Action handlers.
[HttpPost]
public IActionResult ReverseWords(string wordsToReverse)
{
var wordReversalService = _serviceProvider.GetRequiredService<IWordReversalService>();
var reversedWords = wordReversalService.ReverseWords(wordsToReverse);
if (reversedWords == null) throw new NullReferenceException(nameof(reversedWords));
var model = new ReversedArrayModel()
{
InitialArray = wordsToReverse,
ReversedArray = reversedWords
};
return View(model);
}
Now the retrieval and use of the service is localized to a single method.
After doing this to the different service references inside my methods, I feel I am ready to cleanly tease apart this controller into smaller ones, resulting in clear separation of concerns. Could I do this controller decomposition without going through this step of using IServiceProvder? Sure. It just adds to the complexity of moving the methods around the various new controller classes as I massage them into shape.
Again, I am not advocating for solely using IServiceProvider as a way to inject services and then look them up, but I am asserting this is a nice way to start a large controller being refactored into a set of smaller controllers, each with its own intent and purpose.
Check out the [FromServices] parameter attribute.
It’s better to use the [FromServices] attribute. If you really need/want the service provider you don’t need to inject it as it’s always available via HttpContext.RequestServices.
Absolutely true. I got the first note of this from some folks on Mastodon and the follow-up to this post will publish on Thursday.
It was a lot better to use [FromService]
Have you considered using the mediator pattern using a library like MediatR (https://github.com/jbogard/MediatR) to reduce the number of services you are injecting into your controllers? It prevents using the service locator anti-pattern, adds decoupling and adds a request pipeline (which I use to ensure all requests are logged)
Great response. I hadn’t thought of MediaR. Anything from Bogard is worth a look. Will investigate further.