6

I'm writing a C# ASP.NET MVC web application using SOLID principles.

I've written a ViewModelService, which depends on a AccountService and a RepositoryService, so I've injected those two services in the the ViewModelServer.

The PermissionService depends on the HttpContextBase in order to use GetOwinContext() to get an instance of the UserManager. The controller has an instance of HttpContextBase that needs to be used - so it seems like I have to inject the HttpContextBase instance into the ViewModelService which then injects it into the PermissionService.

So, in terms of code I have:

public ViewModelService

public CategoryRepository(ApplicationDbContext context, IPermissionService permissionservice)

public AccountService(HttpContextBase httpcontext, IPrincipal securityprincipal)

to instantiate the ViewModelService, I then do this:

new ViewModelService(
    new CategoryRepository(
            new ApplicationDbContext(), 
            new PermissionService(
                new AccountService(HttpContext, Thread.CurrentPrincipal),
                new UserPasswordRepository(new ApplicationDbContext()),
                new ApplicationSettingsService())),
    new PasswordRepository(
            new ApplicationDbContext(), 
            new PermissionService(
                new AccountService(HttpContext, Thread.CurrentPrincipal), 
                new UserPasswordRepository(new ApplicationDbContext()),
                new ApplicationSettingsService())),
    new ModelValidatorService());

Should a dependency be injected from that many "levels" up, or is there a better way?

marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459
binks
  • 1,001
  • 2
  • 10
  • 26
  • Depends on if you want your deeper classes to be using IoC containers. This is kind of opinion based because some have no problem putting DI code deep in their code and others want to use constructor injection like you have. – TyCobb Dec 15 '14 at 21:19
  • 1
    You might want to consider using a ioc like `unity` or `autofac`. It will prevent the `new` statements and handles a lot of injection-issues for you. – Stefan Dec 15 '14 at 21:19
  • if you use MEF, then you will be abstracting these dependencies and let .net do the injection for you. You can then spend the time you saved buying gifts for your loved ones. – syntax error Dec 15 '14 at 21:21

3 Answers3

5

There's a balance to be struck.

On the one hand, you have the school of thought which would insist that all dependencies must be exposed by the class to be "properly" injected. (This is the school of thought which considers something like a Service Locator to be an anti-pattern.) There's merit to this, but taken to an extreme you find yourself where you are now. Just the right kind of complexity in some composite models, which themselves have composite models, results in aggregate roots which need tons of dependencies injected solely to satisfy dependencies of deeper models.

Personally I find that this creates coupling in situations like this. Which is what DI is intended to resolve, not to create.

On the other hand, you have the school of thought which allows for a Service Locator approach, where models can internally invoke some common domain service to resolve a dependency for it. There's merit to this, but taken to an extreme you find that your dependencies are less known and there's a potential for runtime errors if any given dependency can't be resolved. (Basically, you can get errors at a higher level because consuming objects never knew that consumed objects needed something which wasn't provided.)

Personally I've used a service locator approach a lot (mostly because it's a very handy pattern for introducing DI to a legacy domain as part of a larger refactoring exercise, which is a lot of what I do professionally) and have never run into such issues.

There's yin and yang either way. And I think each solution space has its own balance. If you're finding that direct injection is making the system difficult to maintain, it may be worth investigating service location. Conversely, it may also be worth investigating if the overall domain model itself is inherently coupled and this DI issue is simply a symptom of that coupling and not the cause of it.

David
  • 208,112
  • 36
  • 198
  • 279
  • I've just had a quick look at the Service Locator pattern, and it looks like a ServiceLocator just instantiates a new instance of a dependency to be injected... is this correct? If so, I tend to write a constructor that requires all dependancies, and a constructor that requires no depenancies - this instantiates new instances of the dependancies... would this make a service locator moot? – binks Dec 15 '14 at 21:34
  • @binks: It *can* instantiate, but more often it uses a DI framework to resolve a dependency. (The framework may internally instantiate, may use a singleton instance, may pull from a pool of instances, etc.) I can't imagine having a constructor which instantiates default instances is good for inversion of control as a pattern, because the class is still coupled to *all* of its dependencies in that case. It can be used without them in a logical sense (calling the non-default constructor), but it can't *exist* without them even at compile-time. – David Dec 15 '14 at 21:36
  • @binks: The main difference in patterns is where responsibility is placed. Without service locator, an object says "I don't care where you get it, you need to give me all my dependencies." With service locator, an object says "You don't need to know what my dependencies are, I'm going to get them from someone else when I need them." Is managing that "someone else" more or less complex than managing the graph of dependencies in the objects? That's a judgement call. – David Dec 15 '14 at 21:40
  • 3
    I fall on the side of service locator being an anti-pattern and constructor injection with a single composition root being the One True Way, but it is obviously a bit of a holy war. There *is* a decent discussion on Mark Seemann's original blog post declaring service locator an anti-pattern that's worth reading through: http://blog.ploeh.dk/2010/02/03/ServiceLocatorisanAnti-Pattern/ – Preston Guillot Dec 15 '14 at 21:40
  • @David: This is where I get confused about what DI actually *means*... surely something like my AccountService must depend on HttpContextBase and IPrincipal because without them, it can't do it's default implementation. Or is that what DI means... that you don't need the dependencies, if you choose to override the default implementation? – binks Dec 15 '14 at 21:40
  • 2
    @PrestonGuillot: Seemann makes *a lot* of good points on the subject, and certainly articulates it better than I can. (I also highly recommend his book for anybody serious about DI as more than just adding another framework to the mix.) It often does become a holy war, and in actual business software that war isn't decided by dogmatic proclamations but rather by the bottom line. *If* the domain can be refactored to a simpler model graph, making the question moot, then I'd be a proponent of not using a service locator. That's a big "if" in practical terms though. – David Dec 15 '14 at 21:43
  • 1
    @binks: DI, at its simplest, means that an object shouldn't be responsible for instantiating something it doesn't own. `AccountService` doesn't own `IPrincipal`, so one should be provided to it. The difference in patterns is where it gets that dependency. Should the consuming code supply it (constructor injection), or should another object somewhere else manage all of the dependencies and be queried for it (service locator)? There are pros and cons either way. It's not about overriding a default implementation, because even the default is a dependency that should be injected and not created. – David Dec 15 '14 at 21:46
4

Yes, the entire intent of Dependency Injection is that you compose big object graphs up-front. You compose object graphs from the Composition Root, which is a place in your application that has the Single Responsibility of composing object graphs. That's not any particular Controller, but a separate class that composes Controllers with their dependencies.

The Composition Root must have access to all types it needs to compose, unless you want to get into late-binding strategies (which I'll generally advise against, unless there's a specific need).

Community
  • 1
  • 1
Mark Seemann
  • 225,310
  • 48
  • 427
  • 736
0

I am firmly of the opinion that Service Locators are worse than Dependency Injection. They can be a useful legacy technique, and a useful stepping stone on to something better, but if you are designing something new, then steer clear.

The main reason for this is that Service Locators lead to code that has implicit dependencies, and this makes the code less clear and breaks encapsulation. It can also lead to run time errors instead of compile time errors, and Interacting Tests.

Your example uses Constructor Injection, which is usually the most appropriate form of Dependency Injection:

public ViewModelService(ICategoryRepository categoryRepository, IPasswordRepository passwordRepository, IModelValidatorService modelValidator) { ... }

This has explicit dependencies, which is good. It means that you cannot create the object without passing in its dependencies, and if you try to you will get a compile time error rather than a run time one. It also is good for encapsulation, as just by looking at the interface of the class you know what dependencies it needs.

You could do this using service locators as below:

public ViewModelService() 
{ 
    var categoryRepository = CategoryRepositoryServiceLocator.Instance;
    var passwordRepository = PasswordRepositoryServiceLocator.Instance;   
    var modelValidator  FModelValidatorServiceLocator.Instance;

    ...
}

This has implicit dependencies, that you cannot tell just by looking at the interface, you must also look at the implementation (this breaks encapsulation). You can also forget to set up one of the Service Locators, which will lead to a run time exception.

In your example I thinky your ViewModelService is good. It references abstractions (ICategoryRepository etc) and doesn't care about how these abstractions are created. The code you use to create the ViewModelService is a bit ugly, and I would recommend using an Inversion of Control container (such as Castle Windsor, StructureMap etc) to help here.

In Castle Windsor, you could do something like the following:

container.Register(Classes.FromAssemblyNamed("Repositories").Pick().WithServiceAllInterfaces());
container.Register(Component.For<IAccountService>().ImplementedBy<AccountService>()); 
container.Register(Component.For<IApplicationDBContext>().ImplementedBy<IApplicationDBContext>()); 
container.Register(Component.For<IApplicationSettingsService>().ImplementedBy<IApplicationSettingsService>()); 
var viewModelService = _container.Resolve<ViewModelService>();

Make sure to read and understand the "Register, Resolve, Release" and "Composition Root" patterns before you start.

Good luck!

cedd
  • 1,741
  • 1
  • 21
  • 34