Dependency Injection Optional Parameters

Question

Is it considered a bad practice to use optional parameters when using dependency injection frameworks with Constructor injection?

Example:

public class ProductsController
{
    public ProductsController(IProductService productService = null, IBackOrderService = null)
    {
    }
}

I have specified both parameters as optional, but my DI framework will always inject both dependencies. If I add a new action to my controller which requires a new dependency, would it be bad to make the new dependency optional? I could potentially be breaking dozens of unit tests even though the existing tests wouldn't require the new dependency.

Edit
People seem to be confused by my question. I am never construcing a ProductsController manually in my web application. This is handled by a controller factory (which automatically injects dependencies).

What I don't like is having a unit test like this:

[Test]
public void Test1()
{
    var controller = new ProductsController(new MockProductService(), new MockBackOrderService());
}

Now I decide to add a new action method to my controller. This new action needs a new dependency but none of the existing actions do. Now I have to go back and modify 100 different unit tests because I added a new parameter. I can avoid that by making the parameters optional, but I wanted to know if it was a bad idea. My gut feeling says no because the only thing it affects are the unit tests.

Answer 1

I completely agree with the accepted answer for all the cases on that defining a Dependency means that implementation will not work with out it.

But what if you have something that does not necessarily need a dependency but you want to be able to configure something if that dependency has been loaded. OK...? that sounds a bit weird but its a valid Meta Programming use case - and you think maybe factory pattern could help.. but even the factory may need some, none or all dependencies so Factory does not solve this problem.

Real world problem of feature flags. If a feature is turned off you don't need that dependency.. or maybe cant even create it because there is no concrete implementation. So its turned off. It compiles, everything works OK. But then somebody switches the feature on and all of a sudden we need that dependency.

I found a way to do this -- and the best part is I only realised how to do this by learning about another not so well known technique with Dependency Injection (I am using Microsoft.Extensions.DependencyInjection)

Injecting several concrete implementations for a single Interface

services.AddTransient<IWarehouseRepo, ActionRepository>();
services.AddTransient<IWarehouseRepo, EventRepository>();
services.AddTransient<IWarehouseRepo, AuditRepository>();
services.AddTransient<IWarehouseRepo, ProRepository>();
services.AddTransient<IWarehouseRepo, SuperWarehouseRepository>();
services.AddTransient<IWarehouseRepo, InferiorWarehouseRepository>();
services.AddTransient<IWarehouseRepo, MonthlyStatisticsRepository>();

I only learnt recently that this is completely valid but for this to work your constructor needs to look like this..

public WarehouseHandler(IEnumerable<IWarehouseRepo> repos)

So that is super cool! I can select a repository I need based on whatever criteria. But how does this help with Optional Dependencies?

Because this type of constructor will give you 0 or more dependencies. So if you do not add any dependencies you can branch out in the constructor using a conditional statement

  if (repos.Count() == 0)
    return;

This is null reference safe, does not require default values, easy to debug, easy to read and easy to implement.

You can now also use this technique as the feature switch mechanism too!

Answer 2

I don't think it's a good idea. Constructor injection means that the dependencies are required. You should even add the guard lines that throws if one of the parameters is null.

I think the problem is with your unit tests. For instance I have only one place where the controller is created and supporting objects are mocked (controllerContext, HttpContext, Request, Response, etc.). Then if I add a new parameter in the constructor I have to change it only in one place in the unit tests.

Maybe you should consider to code a generic base class in your unit tests, or make a usage of "setup" routine for the tests.