In my article I want to consider an example of the wrong use of the Dependency Injection principle and try to find motivation for other developers of the team (and maybe someone else still fit) to write new code better, as well as as they run through working activities with someone else’s code, written in an illiterate way, do refactoring.
So, the essence of the problem. On the project, we use OData WebApi and all controllers are inherited from the base class, using the GetService method from the base class, which pulls dependencies through the ApiControllerScopeContextMediator static class.
public abstract class ODataControllerBase : ODataController { protected T GetService<T>() { return ApiControllerScopeContextMediator.GetService<T>(this); } } internal static class ApiControllerScopeContextMediator { internal static T GetService<T>(ApiController controller) { return (T) controller.Configuration.DependencyResolver.GetService(typeof (T)); } }
And in Global.asax we configure dependency pulling for OData through StructureMap:
')
GlobalConfiguration.Configuration.DependencyResolver = new StructureMapDependencyResolver(container);
In all actions at the controllers, the GetService method is commonly used, for example, here:
public class DisconnectedAppsController : ODataControllerBase { public IHttpActionResult Get() { var query = GetService<IQuery<IQueryable<DisconnectedAppDomain>, DisconnectedAppFilter>>(); } }
But why? After all, one could simply use constructor injection:
public DisconnectedAppsController(IQuery<IQueryable<DisconnectedAppDomain>, DisconnectedAppFilter> query){ _query = query; }
So all the same: “Tahiti, Tahiti” (Constructor Injection) or “they feed us well here too” (GetService)?
What problems with this code I see:
- Controllers cannot be covered by unit tests (without initializing the IoC container)
- According to the KISS principle, we do not need additional complexity, but according to YAGNI, an additional link to System.Web.Http. in the beta version has already lost Configuration
- Looking at the class, we obviously do not see all the dependencies of the class.
- Using GetService with a DependencyResolver is essentially using an IoC implementation with the Service Locator , which is an anti-pattern
- and the Service Locator violates the principles of SOLID
- We violate the fundamental principle of OOP encapsulation
What arguments "against" I happened to hear:
- Ideally, controllers should be too stupid to cover them with tests, we will never need this, and if we cover them, then integration tests
- Rejecting the current implementation based on links in Google is too idealistic, it will not be useful.
- Convenient for the developer, less code to write
- Changing the code that is used in this way in many places will introduce entropy into the project.
- YAGNI method in a different plane - but why do we need to change something if there is no obvious every minute benefit
- Constructor injection in controllers trite uncomfortable
A couple of years ago I read the book by Mark Siman
“Dependency Injection” . I sit and think, so what do I have with DI: love or a marriage of convenience?
Used materials:
Mark Seeman "Dependency Injection"Mark Seeman's blogMicrosoft MVC6 github open source projectSOLID wiki pageYAGNI wiki pageKISS wiki page