Thursday, June 02, 2016

Decennial Series #3 – Sitecore.Context is an anti-pattern

And I’ll even attempt to tell you why

If you’re in any way or form following trends for “normal” development (should such a thing exist), you will no doubt have come across one or both of these terms:

  • Inversion of Control
  • Dependency Injection

You might even know what they mean. And why these terms are so popular these days.

Or you might not. But you’re implementing code following these principles anyway. Because your boss, team lead, customer, client or girlfriend demands that you do.

Either way, I’ll just do a very quick explanation of what these mean. If you know enough about these terms to tell me I’m oversimplifying this, this introduction wasn’t meant for you anyway. Skip to the good stuff further down.

IOC and DI in less than 5 minutes

Inversion of Control

Inversion of Control (IoC from here on) is the principle, that nothing in your system should make decisions on exactly which other systems to “talk” to. So your CustomerManager class shouldn’t really get to decide, it wants to talk to a SqlCustomerProvider. Your BasketController shouldn’t really get to decide, everything should be stored in HttpContext.Current.Session.

And why is that?

Because things change. They change over time, as customer requirements change. Someone might later want those BasketItems stored in a back-end Sql table so the Basket isn’t lost between user visits to your site. Someone might not want to pay good money for a Sql Server license to store only 200 customers.

Things change, is my point. And we’re trying to write software to best accommodate that.

But how can I write my BasketController if I can’t store my BasketItems anywhere?

But you can. And in writing it, you do obviously get to decide WHAT you require in order to store and retrieve your BasketItems. You just don’t write the actual implementation. You produce a “blueprint” of the requirements you have.

In C#, we call this an “Interface”.

So in short (simple) summary. Inversion of Control (IoC).

You might have started out like this:

public class BasketController
{
    private readonly BasketStorage _bs;

    public BasketController()
    {
        _bs = new BasketStorage();
    }

    public List<BasketItem> GetBasket(int customerId)
    {
        return _bs.GetBasket(customerId).ToList();
    }

    public void StoreBasket(int customerId, IEnumerable<BasketItem> items)
    {
        _bs.StoreBasket(customerId, items);
    }
}

So what’s the big deal?   Well it’s about control. You might argue; “so what if the client changes her mind, and wants this to come from Sql instead of Session?  I’ll just make a new BasketStorage class, job done”.

And you’re right of course.

But what about me?  the consumer of your API?   or me, the maintenance developer who – 3 years from now – actually want to do both?  Store BasketItems in SessionState – and then persist them to Sql when Session expires?  Why can’t I be your friend?

I’m the consumer of this API, and it’s not unreasonable that I have some form of control over these matters.

So the way we both get happy, is find common ground. With just a little tweaking (and tools like Resharper does like 95% of this work for you anyway; go ask your boss for a license), we can both win. You still get to easily replace one BasketStorage for another, and I get a choice in the matter too.

So we (you, actually) make up the blueprint. The Interface.

public interface IBasketStorage
{
    void StoreBasket(int customerId, IEnumerable<BasketItem> items);
    IEnumerable<BasketItem> GetBasket(int customerId);
}

And you apply this to your existing BasketStorage class.

public class BasketStorage : IBasketStorage
{
    public void StoreBasket(int customerId, IEnumerable<BasketItem> items)
    {
        throw new NotImplementedException();
    }

    public IEnumerable<BasketItem> GetBasket(int customerId)
    {
        throw new NotImplementedException();
    }
}

(and you finish up the methods that I am too lazy to flesh out in this blogpost, naturally)

So far, this is a matter of maybe 20 keystrokes and a few Resharper tricks. And you’re almost done. One last thing.

private readonly IBasketStorage _bs;

public BasketController(IBasketStorage bs)
{
    _bs = bs;
}

And that’s it. You’ve now achieved IoC with Dependency Injection (DI). Well done you.

“But instead of having just 1 place in my code where I created my BasketStorage() class, I am now forced to spread it out everywhere in my codebase!?!?   SURELY that cannot be better?”

And you’re right indeed. Enter: The Service Locator. Notice how I made the person next to you just spill coffee?

How are we for time?  got a couple of minutes before the 5 minutes are up, yea?

Service Locator, also in very short and simple terms.

If you’ve heard the terms IoC and DI, you have absolutely also heard someone – at one point or another – mention “Service Locator is an Anti-Pattern”.

And it can be. Indeed. But let me also tell you…   somewhere in your system, SOMEONE is going to have to instantiate some classes and services at some point. And they are going to need to locate these classes and services. Stay with me on this. We’ll get through this, you and I.

The most direct way to go about, getting these dependencies injected – is to use the Service Locator Pattern. It looks like this:

var basketStorage = DependencyResolver.Current.GetService<IBasketStorage>();
var basketController = new BasketController(basketStorage);

Or the slightly better version:

var basketController = DependencyResolver.Current.GetService<BasketController>();

Notice how, in this last example, we don’t even worry about IBasketStorage any longer. The DependencyResolver is meant to work this out (almost) on it’s own.

“But how does it know?”

Yea, I skipped that didn’t I?  I don’t really want to make this post about, how DI frameworks are configured. In Castle Windsor, it looks like this:

container.Register(Component.For<IBasketStorage>().ImplementedBy<BasketStorage>());

So now, whenever someone requests (registers a dependency for, usually in the class constructor) IBasketStorage, the DependencyResolver knows to instantiate BasketStorage() and send that off to the requesting class.

Just for reference, this form of DI we call “constructor injection” for hopefully obvious reasons.

I think our 5 minutes are up. So let’s address the elephant in the room; “Why is this an Anti-Pattern then?”.

Service Locator and the bad rep

It’s not so bad. Really. Not when used like I just showed here. But that’s not to say, you can’t get in a lot of trouble using SLOC. You absolutely can. Let’s return to the example from before. Imagine I had done it like this:

private readonly IBasketStorage _bs;

public BasketController()
{
    _bs = DependencyResolver.Current.GetService<IBasketStorage>();
}

Same thing, right?  Except now, my BasketController() is nice and easy to instantiate again.

Wrong.

First of all, you’ve now come full circle and achieved exactly nothing. You’ve pushed an interface down on top of BasketStorage, good on you. But to what end?

Dependencies are no longer injected, so DI is out. As for control. Well it’s still somewhat inverted at least – but it’s well hidden from my view. Me, the API consumer/maintenance developer/ravaging psychopath who knows where you live.

So when I, 3 years from now, go… “oh, I’m gonna use this fancy BasketController. Public constructor, no dependencies. I’ll just add 1.000.000 to customerId, and we can use it to store the Wishlish for the customer – I’m a genious. Long weekend, here I come. We’re in an HttpModule with no SessionState, but why should that matter?”. And I go:

var bc = new BasketController();
bc.StoreBasket(1000000+customerId, wishlist);

Yay me. Except. Noooooo. YSOD.

What happened?  Well I didn’t know, invoking BasketController called out using SLOC and procured IBasketStorage. I probably didn’t even know that IBasketStorage required SessionState – but as I said, I didn’t even know about the requirement for it in the first place.

And that, is the problem with the Service Locator.

It hides dependencies. It’s (sort of) IoC but without DI. It’s Prince without The NPG. It’s Bonnie without Clyde.

It’s just not really very good. That’s what I’m saying.

The Good Stuff

I told you this was coming yea?

So as I might have let shine through a little; I do come across zealots from time to time who will turn into an absolute jumblepot of failed arguments and internet quotes whenever I even dare mention the unspeakable Service Locator. “Everybody KNOWS it’s an Anti-Pattern.” (you know who you are).

The irony is, in our day to day work with Sitecore, you (not me) use the Service Locator Pattern ALL THE TIME. Hundreds and hundreds of times every day. Often these very same zealots use it, some of them aren’t even aware.

Take a look at this:

var database = DependencyResolver.Current.GetService<Database>();
var database = Sitecore.Context.Database;

If you think there is anything other than just a semantic difference between those two lines of code, you’ve missed a point somewhere. Start over from the top of this article.

Sitecore.Context is a Service Locator

Actually, I’ll work the font a little and repeat.

Sitecore.Context is a Service Locator

And it has ALL the problems of a Service Locator, too. All of them.

Ever tried calling some of your code – say some Url Generation code – only to find it blows up when called from a Sitecore Publishing Processor?  Sitecore.Context.Site is NOT what you expect is it? 

Or called some Sitecore code from a Unit Test?   I tell you; BOOKS have been written to explain to you why this will blow up. Hint: It’s mostly to do with Sitecore.Context.Database.

Sitecore – in pretty much every corner and crack you look into, has services and functions that rely on other Sitecore services and functions. It is all hidden from view (although some, very far from all, is exposed in the configs), and unless you have lots and lots of Sitecore experience, predicting what calling Sitecore.Context.Database is actually going to need is near impossible.

There is little to no runtime DI in play anywhere; and the config files only allow you to really alter some of the dependencies on a very top level.

And you know what?  this is fine.  Well no it’s not – but it is what it is. If this ever changes, I am almost willing to bet a Dollar, Sitecore will roll out a NEW API to live side-by-side with the existing one. But let’s leave this for now and get to the punchline.

Every time you use Sitecore.Context (or RenderingContext.Current or pretty much any global static in the Sitecore API) – you are using the Service Locator Pattern. And it gives you all the problems that got it labelled as an Anti-Pattern to begin with.

I’m not saying you can avoid it, really. While there are ways to work around most of it; in some cases you just have to cease fire and accept the way things are.

But I’m saying you can:

  • Be aware of it
    • Awareness of a problem is often the first step in minimizing the impact, the problem will have on your project
  • Minimize the use of it
    • Declare your dependencies. Even if you can’t fix all of it; demand “SiteContext context, Database database” in your constructor if you’re going to use them. At least bring back DI.

Over time, this becomes second nature to you. You no longer get mysterious yellow screens because your context is not what you expect. You no longer increase the workload of your poor maintenance programmer by 100-fold, trying to work out what external dependencies your code has (generating a Product Url requires a Site Context, a Context Database, a Context Language, a Context Item and your SSN – seems fair, right? – real story btw, except for the SSN).

Now let’s get back to work.