14

It could seem a stupid question because in my code everything is working, but I've registered a singleton this way with my Unity container _ambientContainer:

 _ambientContainer.RegisterType<Application.StateContext>(new ContainerControlledLifetimeManager());

In order to avoid to use my local field, I use:

get {
    return ServiceLocator.Current.GetInstance<Application.StateContext>();
}

inside my get property to get an instance of my object. This way I get always the same instance (Application.StateContext is still a singleton) or does GetInstance create a new one?

Is it better to use the local _ambientContainer field instead?

get {
    return _ambientContainer.Resolve<Application.StateContext>();
}

Thank you.

Nikhil Agrawal
  • 47,018
  • 22
  • 121
  • 208
zero51
  • 801
  • 2
  • 8
  • 20

3 Answers3

21

Passing around instances of the container to consumer classes isn't generally a good idea, since you are no longer guaranteed to have a single place in your application where components and services are being registered (known as the Composition Root).

Classes should state their dependencies in their public API, ideally by specifying them as constructor arguments, which the container will automatically provide an instance for whenever it's been asked to resolve a specific type (a process known as Autowiring).

Dependency Injection is usually the preferred choice but it isn't always applicable. In those cases using a Service Locator, like you're doing in your example, is the next best solution to decouple a class from its dependencies.

In conclusion, if Dependency Injection is not an option, I would avoid having my classes reference the container directly and instead have them access it through a Service Locator.

Community
  • 1
  • 1
Enrico Campidoglio
  • 56,676
  • 12
  • 126
  • 154
10

Preferably you should avoid both ways of (ab)using your container.

The ServiceLocator is considered an anti-pattern in modern application architecture.

Sebastian Weber
  • 6,766
  • 2
  • 30
  • 49
  • 2
    Do you have a suggestion of an alternative? (specific to the example code in the question?) – Adam Jul 23 '12 at 02:34
  • @Adam An alternative to what code? The getter? Or the registration of `Application.StateContext`? The author did not tell what kind of an application he uses (asp.net/mcv, winforms, wpf, console, wcf ...) so I can't tell you were to put the registration code. Each type of app has a different [composition root](http://blog.ploeh.dk/2011/07/28/CompositionRoot.aspx). As for the call to the service locator: make the `StateContext` a ctor parameter of those classes that need it. That makes the dependency explicit and interchangeable (e.g. for tests). – Sebastian Weber Jul 23 '12 at 06:16
  • 2
    Any pattern is an anti pattern if your using it the wrong way. ALL DI libraries use service location under the covers otherwise they don't work. Don't believe everything you read on the internet! – JBeckton Sep 17 '15 at 20:20
  • 2
    Yes, at some point we have to call an actual implementation class to receive an instance of a type. Frankly, why doesn't Microsoft bake this into System.Activator.CreateInstance ??? I mean shucks, are we all missing a blindingly simple solution here????? – enorl76 Oct 23 '15 at 21:43
6

I'm assuming that the ServiceLocator type is from the CommonServiceLocator project, and that you're using the Unity adapter, in which case GetInstance invokes container.Resolve, so both lines are equivalent.

You can view the source here - http://commonservicelocator.codeplex.com/wikipage?title=Unity%20Adapter&referringTitle=Home

litelite
  • 2,857
  • 4
  • 23
  • 33
devdigital
  • 34,151
  • 9
  • 98
  • 120