ServiceLocator anti-pattern?
I was trying to reply to this article but for some reason I cannot leave a comment there... there's no error message, my comment is simply ignored. So I'm replying here, hopefully I'll be able at some point to at least reply with the address of this article.
Ok... so if I understand this correctly, Nick's problems with the ServiceLocator pattern are:
1) Because it's global, it cannot be tested
2) Sometimes we want to return the same instance for all Resolve() calls, sometimes a thread- or HttpContext-specific instance, sometimes a new instance each time; can't do that with a dictionary
(I'm ignoring the points in the "Problems with..." section because I can't understand any of them. "All of the robustness that is achieved by forcing components to publicly declare dependencies is lost." - why? "When dependencies are retrieved from a global container, the container does not know the identity of the requestor, and is forced to return the same implementation every time." - this is a GOOD thing. You want an instance of X, it shouldn't depend on the requestor - if it does, make many kinds of Xs - it's obviously not the same X. "Concurrency, re-entrancy, circular reference detection, and component lifetime management are much harder/messier in a global container based solution." - again, why?)
It's weird, a real coincidence, but a few days after he wrote this article (which I've only discovered half an hour ago) I wrote a ServiceLocator implementation. This was my first post on the subject, followed by this one and finally this. As shown in the last post, this is one way it can be used:
Can you take a look at this and point out where I'm wrong? I agree, right now I do NOT use any kinds of magic to get rid of the
calls, but I do not actually find that a problem (at least not yet). In fact, if I could find a way that doesn't look forced to replace it with something else, I will probably look into it. (Decorating properties or constructors with [Inject] attributes looks a little weird, but maybe I'm mistaken.)
I've started using this in a project and, so far, everything seems to be OK. I'm definitely open to the idea that problems are lurking in the background though, so I'll appreciate any hint you can give me.
Ok... so if I understand this correctly, Nick's problems with the ServiceLocator pattern are:
1) Because it's global, it cannot be tested
2) Sometimes we want to return the same instance for all Resolve() calls, sometimes a thread- or HttpContext-specific instance, sometimes a new instance each time; can't do that with a dictionary
(I'm ignoring the points in the "Problems with..." section because I can't understand any of them. "All of the robustness that is achieved by forcing components to publicly declare dependencies is lost." - why? "When dependencies are retrieved from a global container, the container does not know the identity of the requestor, and is forced to return the same implementation every time." - this is a GOOD thing. You want an instance of X, it shouldn't depend on the requestor - if it does, make many kinds of Xs - it's obviously not the same X. "Concurrency, re-entrancy, circular reference detection, and component lifetime management are much harder/messier in a global container based solution." - again, why?)
It's weird, a real coincidence, but a few days after he wrote this article (which I've only discovered half an hour ago) I wrote a ServiceLocator implementation. This was my first post on the subject, followed by this one and finally this. As shown in the last post, this is one way it can be used:
- Bind<IProductRepository>
- .To(() => new ProductRepository())
- .CacheBy(InstanceScope.HttpContext);
Can you take a look at this and point out where I'm wrong? I agree, right now I do NOT use any kinds of magic to get rid of the
- var repository = ServiceLocator.GetInstanceOf<IProductRepository>();
calls, but I do not actually find that a problem (at least not yet). In fact, if I could find a way that doesn't look forced to replace it with something else, I will probably look into it. (Decorating properties or constructors with [Inject] attributes looks a little weird, but maybe I'm mistaken.)
I've started using this in a project and, so far, everything seems to be OK. I'm definitely open to the idea that problems are lurking in the background though, so I'll appreciate any hint you can give me.
Comments
Thanks for your feedback on my article. I can see a need for clarifying the points that you're questioning above. They probably deserve more attention than this, but in quick summary:
Publicly declaring dependencies: when dependencies appear as constructor parameters or writeable properties, programmers reusing the class effectively have a specification of what they have to provide in order for that class to work. Without such, they'll have to read source code. This is one of the big motivators for this component-oriented style of programming, because it makes it easier to reconfigure/refactor large applications.
Different implementations for different requestors: If I start building an app that has only one database connection, then all the components ask for "connectionString". If I then include some components in from another app, that ask for "connectionString" but really mean an MSMQ server connection - with dependency injection I'm fine, with service locator I have to change code. A bigger issue when the services are identified by type (e.g. IFoo).
Concurrency, re-entrancy, etc.: take circular dependency detection as an example - if A calls the SL for B, and B calls the SL for A, there is a question of where to keep the state that describes this so that an error other than StackOverflowException can be thrown. The other items relate to similar issues - nothing's ever impossible, but it can get tough.
Hope this helps, thanks again for dropping by.
Nick
1. Publicly declaring dependencies. Absolutely necessary, except that the framework won't usually tolerate this, at least in ASP.NET. So what I end up with is this:
public class ProductRepository : IProductRepository
{
private readonly IRepository<Product> repository;
public ProductRepository()
: this(ServiceLocator.GetInstanceOf<IRepository<Product>>())
{
}
public ProductRepository(IRepository<Product> repository)
{
this.repository = repository;
}
This way I get the best of both worlds: the caller knows the dependency and can use that constructor (from a test case for example), but the framework can use the default constructor and the ServiceLocator will still retrieve the correct instance.
2. Different implementations for different requestors: I haven't hit this problem, I must admit. I have no idea how I could solve a ServiceLocator.GetInstanceOf<string>() call. I am, however, unclear on how a DI container can solve it either.
It would seem to me that identifying by type would be easier... I can ask for a ILocalFoo or a IRemoteFoo, even if they're basically identical. Agreed, it does look like a hack, but again, how exactly do you tell the DI container "when X asks for resource R, give it a R1, when Y does the same thing, give it a R2"?
3. Circular dependencies: I don't see any reason for a stack overflow, unless A needs a new instance of B, and then in turn B needs a new instance of A and so on. If they are singletons, for example, you bind the actual types in the main() or global.asax, not when you're using them:
Bind<IB>.To(() => new A()).CacheBy(InstanceScope.Singleton);
Bind<IA>.To(() => new B()).CacheBy(InstanceScope.Singleton);
A needs an instance of IB:
var ib = ServiceLocator.GetInstanceOf<IB>();
B needing an instance of IA is similar. No stack overflow.
Different implementations for different requestors are configured at the level of the requestor, so that would still require a change in the Bind statement - just not a change in the component. The different implementations need to be configured with different keys, and the bindings for the client components each need to use a different key.
If:
var ib = ServiceLocator.GetInstanceOf<IB>();
is in the constructor of A, and a similar call is in the constructor of B, I think a stack overflow will still occur in this example.
Even stack overflow for new instances is a problem however, because StackOverflowException is a nasty exception to recover from.
Cheers!
Nick
I guess I'll have to study the DI frameworks some more. They still smack of magic a bit much, and I still absolutely hate the idea of XML configuration, but I guess I'll have to bite the bullet sooner or later :)
Thanks for discussing this!