Dependency Injection: Inject Your Dependencies, Period!
Reasons on why I prefer dependency injection over static accessors.
18 November 2014
4 minutes read
Related Posts
There is a discussion going on inside ASP.NET/Logging repository whether we should have static Logger available everywhere or not. I am quite against it and I stated why with a few comments there but hopefully with this post, I will try to address why I think the dependency injection path is better for most of the cases.
Let’s take an example and explain it further based on that.
public class FooManager : IFooManager { private readonly IClock _clock; private readonly ILogger _logger; private readonly IEnvironmentInfo _environmentInfo; public FooManager(IClock clock, ILogger logger, IEnvironmentInfo environmentInfo) { _clock = clock; _logger = logger; _environmentInfo = environmentInfo; } // ... }
This is a simple class whose job is not important in our context. The way I see this class inside Visual Studio is as below:
In any other text editor:
Inside Visual Studio when I am writing tests:
There are two things fundamental things why I would like this approach.
The Requirements of the Component is Exposed Clearly
When I look at this class inside any editor, I can say that this class cannot function properly without IClock, ILogger and IEnvironmentInfo implementation instances as there is no other constructor (preferably, I would also do null checks for constructor parameters but I skipped that to keep the example clean). Instead of the above implementation, imagine that I have the following one:
public class FooManager : IFooManager { public void Run() { if(DateTime.UtcNow > EnvironmentInfo.Instance.LicanceExpiresInUtc) { Logger.Instance.Log("Cannot run as licance has expired."); } // Do other stuff here... } }
With this approach, we are relying on static instances of the components (and they are possibly thread-safe and singleton). What is wrong with this approach? I’m not sure what’s the general idea for this but here are my reasons.
First thing for me to do when I open up a C# code file inside visual studio is to press CTRL+M, O to get an idea about the component. It has been kind of an habit for me. Here how it looks like when I do that for this class:
I have no idea what this class needs to function properly. I have also no idea what type of environmental context it relies on. Please note that this issue is not that big of a problem for a class which is this simple but I imagine that your component will have a few other methods, possible other interface implementations, private fields, etc.
I Don’t Need to Look at the Implementation to See What It is Using
When I try to construct the class inside a test method in Visual Studio, I can super easily see what I need to supply to make this class function the way I want. If you are using another editor which doesn’t have a tooling support to show you constructor parameters, you are still safe as you would get compile errors if you don’t supply any required parameters. With the above static instance approach, however, you are on your own with this issue in my opinion. It doesn’t have any constructor parameters for you to easily see what it needs. It relies on static instances which are hard and dirty to mock. For me, it’s horrible to see a C# code written like that.
When you try to write a test against this class now, here is what it looks like if you are inside Visual Studio:
You have no idea what it needs. F12 and try to figure out what it needs by looking at the implementation and good luck with mocking the static read-only members and DateTime.Now :)
I’m intentionally skipping why you shouldn’t use DateTime.Now directly inside your library (even, the whole DateTime API). The reasons are variable depending on the context. However, here are a few further readings for you on this subject:
It is Still Bad Even If It is not Static
Yes, it’s still bad. Let me give you a hint: Service Locator Pattern.
Hey, I'm Tugberk 👋
Coder 👨🏻💻, Speaker 🗣, Author 📚, ex-Microsoft MVP 🕸, Blogger 💻
I live in London, UK 🏡
