Would Injecting dependencies in C# as default parameters be a bad practice?

Question

Given the (old) debate over whether Singletons are overused/abused/are worth it - would it be a bad idea to inject the dependencies as default parameters? In this way, we could get rid of defining singletons for the dependency container registry for the corresponding interfaces - while still leaving the objects in question testable - all one needs to do is to explicitly inject a mock as the dependency when Unit Testing such classes.

Example:

public class Foo
{
    IDoSomething x;
    Foo(IDoSomething x = null)
    {
        this.x = x ?? new DoSomething()
    }
}

public class DoSomething : IDoSomething
{
   ...
}

in Program.cs
services.AddSingleton(IDoSomething, DoSometing);

This way I am rid of holding singletons throughout my app, without losing testability.

public class FooTests
{
    Foo SUT;
    Mock<Foo> FooMock;
    FooTests() 
    {
        FooMock = new Mock<Foo>();
        FooMock.Setup(...);
        SUT = new Foo(FooMock.Object);
    }
}

Giving it a second thought, if I require a singleton (if I need to assure no 2 instances exist for this object) than it's more like a matter of defining the lifetime of that instance (singleton, scoped, transaction). In other words, there should be nothing wrong in injecting a dependency as a mandatory parameter - and properly registering the instantiation type at the dependency container.

Answer 1

In any reasonably sized codebase, the nested DI parameters are going to explode, which is going to bite you when you want to write those concrete constructors like that. Even if reasonable on a whiteboard, tyour idea does not pass the smell test in actual production code.

Based on that alone, it's probably worth letting go of this idea.

If you're going to be using singletons (which I advise against but never mind that), using a DI container is a better way to do so.

However, that doesn't explain why you're trying to make these optional parameters. What would be the benefit of deciding the concrete type DoSomething in the constructor, as opposed to deciding it when registering your DI container?

If multiple classes depend on the same dependency, you have to repeat the same concrete class over and over.

I don't see any upside to this. It seems to me that the only thing you're achieving is that you sidestep the DI container in favor of manually mapping (and tightly coupling) the concrete dependency types. That's a step in the wrong direction.

Answer 2

First things first: what you have heard is correct, the Singleton pattern is an anti-pattern that is often abused to gain what the pattern considers it's "cost" or "disadvantages", but being able to deflect blame in a code review and say "oh, but it's the pattern".

But the "Singleton Lifetime" is not the same as the "Singleton Pattern". Specifically, it does not have the same drawbacks and is therefore not abused by people this way.

So in short, using Singleton Lifetime in your Dependency Injection Container is perfectly fine.

So your actual question: you can do this, but it is it worthwhile? You obviously get some of the advantages of dependency injection, because you can inject dependencies. However, you still get tight coupling between your components, because they know each other.

I have used this "pattern" in the past, when I had no use for a full dependency injection container, because each solution would only contain three classes. And yes, those were coupled tightly this way. But it was three. Very clearly laid out. But in turn we had about a hundred of those assemblies. Putting in a hundred dependency injection containers, each with exactly 2 registrations would only have complicated things.

So to summarize: normally, you should not do this. Not because Singleton is an anti-pattern (it is, but it is not used here), but because you do not get the full benefits since you still have very tight coupling of components. Only if you are sure you do not need this benefit, you can do this. And it will work well for those rare cases. If in doubt, you probably don't have such a case.

Answer 3

There's no singleton in your example, but what you are doing isn't bad per se.

Its fairly common practice to provide default implementations where there is going to be tight coupling regardless. Obviously you are not quite as decoupled, but as long as the default class is small the downside is small or nothing and you gain convenience of use.