Thursday, May 15, 2014

Service Locators and the Interface Segregation Principle

Okay, it has been a long time since I've posted here, and boy have I learned a lot in the years since my last post! A co-worker has been encouraging me to start writing blog posts about programming, and in a conversation on Google Plus, Mark Seemann suggested that I should put some of my thoughts in a blog post, so here goes:

Mark Seemann recently blogged (again) about the Service Locator anti-pattern. He tried to describe why it's an antipattern in terms of the Interface Segregation Principle:

From a client's perspective, there no limit to how many variations of the Create method it can invoke:
var foo = serviceLocator.Create<IFoo>();
var bar = serviceLocator.Create<IBar>();
var baz = serviceLocator.Create<IBaz>();
// etc.Again, from the client's perspective, that's equivalent to multiple method definitions like:
IFoo CreateFoo();
IBar CreateBar();
IBaz CreateBaz();
// etc.
However, the client can keep coming up with new types to request, so effectively, the number of Create methods is infinite!

This argument goes part-way there, but needs a little more fleshing out. After all, you could say that any method with parameters can be expanded out in this way:

var x = service.Foo(1);
var y = service.Foo(2);
var z = service.Foo(3);
// etc.

corresponds to

var x = service.Foo1();
var y = service.Foo2();
var z = service.Foo3();
// etc.

The key difference is that programmers almost never write code like what's in the example above: what's up with those magic numbers? We generally say `var a = Foo(x)`, where `x` is somehow derived from the method's input. That means you can write a meaningful unit test that ensures a given `x` gets passed into `Foo()` when you provide a given `y` as a parameter. This aspect of your method is unlikely to change as you modify implementation details--or if it does, it will probably be the sort of change which will require the test to be updated before it compiles.

On the other hand, if you're using a service locator, you're effectively passing hard-coded "magic classes" into the method all over the place. The values you pass in are rarely based on the input provided to the method, and can easily change as you refactor, without forcing anyone to recognize that your method now makes use of a different "interface" method, which needs to be mocked up before the test will run.

The fact that the intended usage of the interface involves passing in hard-coded values that are not defined by an `enum` indicates that you are, in essence, creating an infinite number of different "ways" to call methods on your interface. If you subscribe to the notion of "role-based" interfaces (which I do), then the usage of the interface should be what informs how the interface is written, and if there are a large number of ways for calling code to invoke a method on your interface, it creates the same problems as having a large number of methods on the interface.

What about Generics?

"Okay," you may be saying, "but wouldn't that make this an argument against all generics? After all, you can pass any type you want into your `Maybe.Not<>()` method, and generic parameters are hard-coded by definition: isn't that an anti-pattern?

The difference is that the behavior of a `List<>` constructor, or `Maybe.Not<>()` is exactly the same no matter what type gets passed in. These are data structures, not service methods. 

On the other hand, if you're talking about something like Automapper's `Mapper.Map<,>()` method, its behavior is going to vary widely based on the generic type arguments, and I'd suggest that this is, indeed, an anti-pattern. Your call to AutoMapper will be working fine one day, and then fail the next just because you renamed a property somewhere, and no compiler ever gave you so much as a hint that you broke your mapping.

I've got more opinions to offer on AutoMapper, but now's not the time. Hopefully this was enlightening.