Catel newbie question: Isn't the ServiceLocator pattern an Anti-pattern?

Topics: Feature requests
Nov 6, 2012 at 10:48 AM

Hi to all

I'm new to Catel , but I'm using a custom MVVM framework in a WPF app that I'd like to convert to use an out of the box framework.

I'm especially interested in implementing a DI pattern as I'm using a static factory pattern now...

I've seen several good posts about Catel. Diving in the docs I've seen that it seems to use the ServiceLocator pattern deeply, but I'm strongly against its use like in the Message mediator doc article:

var mediator = ServiceLocator.Instance.ResolveType<IMessageMediator>();
mediator.Register<string>(this, OnMessage);

This introduces the problems described for example here: http://www.devtrends.co.uk/blog/how-not-to-do-dependency-injection-the-static-or-singleton-container and here: http://blog.ploeh.dk/2010/02/03/ServiceLocatorIsAnAntiPattern.aspx

What's your position in this anti-pattern debate? Is there a practical way to prevent this using Catel?

Thanx 
Paolo 

Coordinator
Nov 7, 2012 at 9:06 AM

Hi Paolo,

Excellent question. We have also discussed this internally, and we think calling it an anti-pattern is an anti-pattern ;-). Let me explain a very common situation and why we still choose to use the ServiceLocator.

Constructor injection
Constructor injection is a really good way to inject dependencies into a class. However, think about a very common scenario. You have a view model for a window. It needs the IPleaseWaitService so you inject it into the window. Then, you create a nested view model. This one needs the IMyDataProvider. Now what, are you going to inject *all* child view model dependencies in the window view model? Aren't you moving the responsibility of the dependency container from the IoC container to the main view model?

Other methods of avoiding the ServiceLocator
There are other ways of avoiding the ServiceLocator. Good examples are MEF and Unity. While Unity still allows you to set attributes such as [ImportConstructor]. MEF is even more automatic and hooks everything together magically with the cost of a major performance hit. But wait, didn't you create an anti-pattern here by using Unity or MEF in the first place? Yes, you did.

Best of both sides
So as I said earlier, constructor injection is a really good way to create objects. Catel does support this. So, if you have an object with dependency injection, you can simply resolve it from the ServiceLocator.ResolveType<> or use TypeFactory.Default.CreateInstance. The TypeFactory will use the service locator to retrieve dependencies. Each service locator has it's own TypeFactory and the TypeFactory is also registered inside the service locator (you can retrieve it using ResolveType<ITypeFactory>();

This means that if you have an object with multiple dependencies, you can use DI. The only downside at the moment is that we haven't found a good way to allow the combination of nested user controls with datacontext injection and DI. We will think of a method in the future.

An in-between what's possible is that you inject the IServiceLocator as a dependency. This way, you can create your own service locator instances and pass them around.

Nov 7, 2012 at 10:10 AM

Hi

thanx for your exhaustive response!

I'll need some time to test all the implications of the above points in my own solution...

For the moment I'll try with the last option, injecting the IServiceLocator as a dependency, and I'll see how it fits. 

QUESTION: do u know of any implications in propagating this single dependency down to child view models with Catel?

 

Indeed I personally think that the ServiceLocator pattern used without knowing its risks IS an anti-pattern...

My primary concern is the testability of the classes that uses the ServiceLocator pattern, and I think this is the main concern that causes so many people calling it an "anti-pattern".
For example, if I use the ServiceLocator in a method to locate and instantiate a service with some interface, and I simply use it and then dispose it, I'm hiding this dependency in my internal implementation. Imagine doing this in hundreds classes and with hundreds services..
There isn't an explicit contract between my class and the external world, so how can u assure your testing coverage? The answer is: you have to constantly read all the source code of the class to gather all the dependencies it has!

MEF and Unity, on the other hand, gives you the opportunity to move the construction of the dependency graph in a single location (for WPF at startup), so it's not an anti pattern at all! In fact you didn't have to go through ALL your classes to see which are the dependencies, 'cause you have a single place to look at, and your testing experience will benefit from this...

So I think that Constructor injection is the simplest and the clearest solution to do DI.. A clear contract between your classes and the rest of the world.
As for the child view model issues, an option could be not using them at all, like in other frameworks. A single view model is responsible for all its child views.
But maybe you'll find a workable solution in the future to propagate dependencies down to children in a semi automatic way (for example using some sort of attributes on the parent class to tag the dependencies that needs to propagate down). 

In conclusion, I think that, in a TDD world, you have to stay away from solutions that hides the full set of responsibilities of a class!
Another bad example are loose messaging systems, when a view model can send arbitrary messages out in the space, and other view models register to receive them.. how do u think your test experience will be?
A simple and workable solution here is to EXPLICITLY implement an interface on the receiver that declares all the messages entry points as methods (methods ARE messages, aren't they?)
During runtime you continue to use the messaging system, BUT in your tests you mock out the messenger service and simply use the plain old methods to send messages to your view models...

I'm sorry for the length of this post, but I hope that this clarifies the whole point of ServiceLocator detractors! ;)

Paolo

Coordinator
Nov 7, 2012 at 4:10 PM

I have always considered messaging a "dirty way" out of an issue that you haven't solved right from the beginning. But... some people like to use it because it is so easy (those are the same people that don't write test code), so lets conclude that we think the same about messaging :-)

I agree that in a perfect world, all dependencies are injected into a class using a constructor. This way, you are not bound to a specific technology (such as MEF, Unity, whatsoever), but you can simply inject them (manually or automatically using MEF, Unity, TypeFactory).

Unfortunately, we do not live in a perfect world. The fact that you want to create all view models doesn't solve anything. With that I mean: if I add a dependency in a 3n deep level of VM's, I have to add the dependency to all view models above it and keep passing the values through. Then, the view models become some sort of service locators them selves which is bad IMHO.

The service locator in catel is instantiatable. You can inject service locators into view models. This means that you can create your own service locator with the right data for every test. If you are running a real application, you register all your services / dependencies at startup in the service locator and the app will get them if needed. Again, here you have a single point that is responsible for the actual dependency injection / resolving: the configured service locator.

I agree with you on the fact that the service locator hides dependencies and that this *might* make it all a bit more complex. The issue here is: what's worse: passing dependencies around for usage in child objects of child objects, or "assuming" or "knowing" that a dependency is registered at startup.

If you are writing tests, you *know* what you are testing. You *know* the dependencies and you should be able to create a new instance of the ServiceLocator with the right registered (test) services. Maybe it's not the best solution, but I think that passing dependencies for the sake of dependency injection is wrong as well.

Nov 7, 2012 at 6:06 PM

I see your point here, and you're absolutely right.. Using the ServiceLocator pattern is not an anti-pattern as long as you properly document your code.

IMHO it's better to always inject the service locator as a dependency into view models. It's a balance between the constructor injection pattern and the use of ServiceLocators as Gods objects (in the sense that they materialize instances of services in your classes without an explicit contract).

Speaking of loosely coupled messaging pattern... I think it has its way to go, at least when nothing else works out of the box...
In my experience, I faced the situation in which I wanted to communicate with other view models, but I didn't know the concrete type of my recipents, only the public interface. And I didn't know HOW MANY of them were instantiated at one time.
For example if I have a pluggable system where some view models are slightly different implementations of a common functionality, and I want to broadcast a message to all (how many?) of them, how can I accomplish this using Catel, other than using MessageMediator?

In Catel (as I read in the docs) it's possible to decorate a ViewModel with the InterestedInAttribute... but can I use it with an interface type instead of a concrete type? Catel should then notify me of changes occurring in ALL the view models implementing that interface. (here using multiple attributes with concrete types isn't an option, as I'm building an open pluggable system).
If it's not supported, I honestly can't see other ways of implementing communication between view models in the above scenario.. maybe creating some custom service that implements a highly specialized mediator pattern?

I'd be very happy to hear your opinion on this scenario, as I'm really struggling with this.. :)

Coordinator
Nov 7, 2012 at 6:27 PM

You can either use the InterestedIn or the MessageMediator. However, if you want to do it really good, you could add a manager for the interfaces and call each interfaces. The InterestedIn is an automated system for this. The MessageMediator is a really dirty way to throw information around, which I think is worse than the InterestedIn.

In the ViewModelBase, the retrieval of the InterestedInAttribute allows inherited members to be used. This allows you to put the attribute on a base and all inherited view models will be notified as well. Interfaces will not work in the current implementation. The items are retrieved by type, and if you pass in a type, the instance.GetType() will always return the final type, not an interface.

It might be possible to implement this in the future, but I really want 3.4 out so not for the current release.

If you create the unit tests and describe the behavior (or simple create a demo app that you *expect* to work in the new way), I will implement it as a feature first thing after the 3.4 release. Then you will have this feature in about a week and you can use the nightly build and you can happily use the feature.

Nov 8, 2012 at 2:12 PM

Thanx Geert,

I really appreciate the time and effort you put in helping devs out there! :)

For now I'll try to port my codebase piece after piece using Catel, and trying to improve my overall architecture. When I'll try to overcome the messenger part I'll see if the InterestedInAttribute will fit fine, or a custom manager will fit best...

 

As for the DI in view models, I've decided to stick with constructor injection. I prefer verbose interfaces (maybe it's related to my C roots.. :P)

Speaking of the nested user controls issue, I've taken a look to your codebase. Do u think that, extending the ViewModelFactory to take advantage of the TypeFactory powers, could solve the construnctor based DI for nested view models?

Imagine I have a ViewModel with this constructor:

 

public CompanyViewModel(Models.Company company, ISomeService service1, ISomeOtherService service2)
   : base()
{
  // Store values
   
}

In the ConstructViewModelUsingArgumentOrDefaultConstructor of LogicBase you use the ViewModelFactory to create a concrete view model type, and if the CreateViewModel method could dynamically resolve other parameters of view model constructor using service locator (what TypeFactory does) u could resolve the problem of auto instantiating view models for nested views..

Hope this makes some sense... ;)

Thx & keep up the great work!
Paolo 

Coordinator
Nov 8, 2012 at 4:50 PM

That's exactly what I was thinking of. Just need to find the time to implement it ;-)