Refactoring from Service Locator to Abstract Factory by Mark Seemann
One of the readers of my book recently asked me an interesting question that relates to the disadvantages of the Service Locator anti-pattern. I found both the question and the potential solution so interesting that I would like to share it.
In short, the reader's organization currently uses Service Locator in their code, but don't really see a way out of it. This post demonstrates how we can refactor from Service Locator to Abstract Factory. Here's the original question:
"We have been writing a WCF middle tier using DI
"Our application talks to multiple databases. There is one Global database which contains Enterprise records, and each Enterprise has the connection string of a corresponding Enterprise database.
"The trick is when we want to write a service which connects to an Enterprise database. The context for which enterprise we are dealing with is not available until one of the service methods is called, so what we do is this:
public void MyServiceOperation(
EnterpriseContext context)
{
/* Get a Customer repository operating
* in the given enterprise's context
* (database) */
var customerRepository =
context.FindRepository<Customer>(
context.EnterpriseId);
// ...
}"I'm not sure how, in this case, we can turn what we've got into a more pure DI system, since we have the dependency on the EnterpriseContext passed in to each service method. We are mocking and testing just fine, and seem reasonably well decoupled. Any ideas?"
When we look at the FindRepository method we quickly find that it's a Service Locator. There are many problems with Service Locator, but the general issue is that the generic argument can be one of an unbounded set of types.
The problem is that seen from the outside, the consuming type (MyService in the example) doesn't advertise its dependencies. In the example the dependency is a CustomerRepository, but you could later go into the implementation of MyServiceOperation and change the call to context.FindRepository<Qux>(context.EnterpriseId) and everything would still compile. However, at run-time, you'd likely get an exception.
It would be much safer to use an Abstract Factory, but how do we get there from here, and will it be better?
Let's see how we can do that. First, we'll have to make some assumptions on how EnterpriseContext works. In the following, I'll assume that it looks like this - warning: it's ugly, but that's the point, so don't give up reading just yet:
public class EnterpriseContext
{
private readonly int enterpriseId;
private readonly IDictionary<int, string>
connectionStrings;
public EnterpriseContext(int enterpriseId)
{
this.enterpriseId = enterpriseId;
this.connectionStrings =
new Dictionary<int, string>();
this.connectionStrings[1] = "Foo";
this.connectionStrings[2] = "Bar";
this.connectionStrings[3] = "Baz";
}
public virtual int EnterpriseId
{
get { return this.enterpriseId; }
}
public virtual IRepository<T> FindRepository<T>(
int enterpriseId)
{
if (typeof(T) == typeof(Customer))
{
return (IRepository<T>)this
.FindCustomerRepository(enterpriseId);
}
if (typeof(T) == typeof(Campaign))
{
return (IRepository<T>)this
.FindCampaignRepository(enterpriseId);
}
if (typeof(T) == typeof(Product))
{
return (IRepository<T>)this
.FindProductRepository(enterpriseId);
}
throw new InvalidOperationException("...");
}
private IRepository<Campaign>
FindCampaignRepository(int enterpriseId)
{
var cs = this.connectionStrings[enterpriseId];
return new CampaignRepository(cs);
}
private IRepository<Customer>
FindCustomerRepository(int enterpriseId)
{
var cs = this.connectionStrings[enterpriseId];
return new CustomerRepository(cs);
}
private IRepository<Product>
FindProductRepository(int enterpriseId)
{
var cs = this.connectionStrings[enterpriseId];
return new ProductRepository(cs);
}
}
That's pretty horrible, but that's exactly the point. Every time we need to add a new type of repository, we'll need to modify this class, so it's one big violation of the Open/Closed Principle.
I didn't implement EnterpriseContext with a DI Container on purpose. Yes: using a DI Container would make it appear less ugly, but it would only hide the design issue - not address it. I chose the above implementation to demonstrate just how ugly this sort of design really is.
So, let's start refactoring.
Step 1 #
We change each of the private finder methods to public methods.
In this example, there are only three methods, but I realize that in a real system there might be many more. However, we'll end up with only a single interface and its implementation, so don't despair just yet. It'll turn out just fine.
As a single example the FindCustomerRepository method is shown here:
public IRepository<Customer>
FindCustomerRepository(int enterpriseId)
{
var cs = this.connectionStrings[enterpriseId];
return new CustomerRepository(cs);
}
For each of the methods we extract an interface, like this:
public interface ICustomerRepositoryFinder
{
int EnterpriseId { get; }
IRepository<Customer> FindCustomerRepository(
int enterpriseId);
}
We also include the EnterpriseId property because we'll need it soon. This is just an intermediary artifact which is not going to survive until the end.
This is very reminiscent of the steps described by Udi Dahan in his excellent talk Intentions & Interfaces: Making patterns concrete. We make the roles of finding repositories explicit.
This leaves us with three distinct interfaces that EnterpriseContext can implement:
public class EnterpriseContext :
ICampaignRepositoryFinder,
ICustomerRepositoryFinder,
IProductRepositoryFinder
Until now, we haven't touched the service.
Step 2 #
We can now change the implementation of MyServiceOperation to explicitly require only the role that it needs:
public void MyServiceOperation(
ICustomerRepositoryFinder finder)
{
var customerRepository =
finder.FindCustomerRepository(
finder.EnterpriseId);
}
Since we now only consume the strongly typed role interfaces, we can now delete the original FindRepository<T> method from EnterpriseContext.
Step 3 #
At this point, we're actually already done, since ICustomerRepositoryFinder is an Abstract Factory, but we can make the API even better. When we consider the implementation of MyServiceOperation, it should quickly become clear that there's a sort of local Feature Envy in play. Why do we need to access finder.EnterpriseId to invoke finder.FindCustomerRepository? Shouldn't it rather be the finder's own responsibility to figure that out for us?
Instead, let us change the implementation so that the method does not need the enterpriseId parameter:
public IRepository<Customer> FindCustomerRepository()
{
var cs =
this.connectionStrings[this.EnterpriseId];
return new CustomerRepository(cs);
}
Notice that the EnterpriseId can be accessed just as well from the implementation of the method itself. This change requires us to also change the interface:
public interface ICustomerRepositoryFinder
{
IRepository<Customer> FindCustomerRepository();
}
Notice that we removed the EnterpriseId property, as well as the enterpriseId parameter. The fact that there's an enterprise ID in play is now an implementation detail.
MyServiceOperation now looks like this:
public void MyServiceOperation(
ICustomerRepositoryFinder finder)
{
var customerRepository =
finder.FindCustomerRepository();
}
This takes care of the Feature Envy smell, but still leaves us with a lot of very similarly looking interfaces: ICampaignRepositoryFinder, ICustomerRepositoryFinder and IProductRepositoryFinder.
Step 4 #
We can collapse all the very similar interfaces into a single generic interface:
public interface IRepositoryFinder<T>
{
IRepository<T> FindRepository();
}
With that, MyServiceOperation now becomes:
public void MyServiceOperation(
IRepositoryFinder<Customer> finder)
{
var customerRepository =
finder.FindRepository();
}
Now that we only have a single generic interface (which is still an Abstract Factory), we can seriously consider getting rid of all the very similarly looking implementations in EnterpriseContext and instead just create a single generic class. We now have a more explicit API that better communicates intent.
How is this better? What if a method needs both an IRepository<Customer> and an IRepository<Product>? We'll now have to pass two parameters instead of one.
Yes, but that's good because it explicitly calls to your attention exactly which collaborators are involved. With the original Service Locator, you might not notice the responsibility creep as you over time request more and more repositories from the EnterpriseContext. With Abstract Factories in play, violations of the Single Responsibility Principle (SRP) becomes much more obvious.
Refactoring from Service Locator to Abstract Factories make it more painful to violate the SRP.
You can always make roles explicit to get rid of Service Locators. This is likely to result in a more explicit design where doing the right thing feels more natural than doing the wrong thing.
