Tuesday, April 17, 2012

Some Servicelocator pattern stinks

I have been working on a somewhat legacy codebase which makes use of the Servicelocator pattern. Although I always thought of Dependecy Injection to be the superior pattern, I was pleased to find some Inversion of Control implementation in there. Working with the codebase, I discovered first hand how easily, when used without caution and discipline, the Servicelocator pattern can introduce code rot.

I will walk you through some of the issues I have with the Servicelocator pattern, mostly looking at it from a test perspective. It's interesting how you can often quickly discover friction in a codebase by just looking at, or writing, tests.

The first thing that rubs me the wrong way is that by using the Servicelocator pattern, you make each class dependant on the servicelocator, degrading the purity of the models. Although this is a conceptual consideration, it considerably affects development.

What I really like about Dependency Injection is that you can look at the constructor (or properties) from a class and instantly know its dependencies. The Servicelocator pattern makes it too easy to hide them.

To demonstrate some of the beef I have with the Servicelocator pattern, I wrote a FireStation class which has one public method Alert. I chose an example which is not the de facto OrderService or ShoppingBasket implementation, but which still is small enough to grasp easily.

So let's have a look at this FireStation class. I know it's a ridiculously naïve implementation, but good enough as an example.
public class FireStation
{
    public void Alert(Incident incident)
    {
        SendPagerMessage();
        SendEmail();

        if (incident.Priority == IncidentPriority.High)            
            ActivateSiren();                        
    }

    private void SendPagerMessage()
    {
        ServiceLocator.Current.GetInstance<IPagingTerminal>().SendMessage();
    }

    private void SendEmail()
    {
        ServiceLocator.Current.GetInstance<IMailServer>().SendMailMessage();
    }

    private void ActivateSiren()
    {
        ServiceLocator.Current.GetInstance<IImmoticaServer>().ActivateSiren();
    }
}
There is one public Alert method, which takes an Incident instance, and uses three alarmation channels to alert firemen: paging, email and a siren.

So, let's imagine I wanted to test whether the siren is activated when there is a high priority incident. I would just start writing the test, to ultimately find out I have no idea what to assert.
[TestMethod()]        
public void Alert_activates_the_siren_when_the_priority_is_high()
{
    var fireStation = new FireStation();

    fireStation.Alert(new Incident(IncidentPriority.High));

    Assert.Inconclusive("How can I verify the siren is activated?");
}
Looking at the collapsed method definitions, I still don't know. There is no constructor, so the dependencies aren't resolved in there. Nothing left to do but inspect the Alert method content, and look at the ActiviateSiren method implementation. That's where I eventually find out I need to mock the IImmoticaServer interface to assert the interaction.

So I do that, by creating the mock, setting up the container and setting it as the provider for my servicelocator.
[TestMethod()]    
public void Alert_activates_the_siren_when_the_priority_is_high()
{            
    var immoticaServer = new Mock<IImmoticaServer>();

    var kernel = new StandardKernel();
    kernel.Bind<IImmoticaServer>().ToConstant(immoticaServer.Object);
    
    ServiceLocator.SetLocatorProvider(() => new NinjectServiceLocator(kernel));    

    new FireStation().Alert(new Incident(IncidentPriority.High));

    immoticaServer.Verify(i => i.ActivateSiren(), Times.Once());
}
Now I'm polluting my test with responsibilities it shouldn't really care about. I could probably move that into some infrastructure or the test setup, but still, I'm writing code that could be easily averted.

Anyways, I should now be able to verify the interaction with the IImotticaServer implementation. Boom, red test. You probably figured this one out already, but if I hadn't expanded the private methods, you would have had no idea that I needed two extra stubs to make the test pass; one for the IPagingTerminal dependency and one for the IMailServer dependency.
kernel.Bind<IPagingTerminal>().ToConstant(new Mock<IPagingTerminal>().Object);
kernel.Bind<IMailServer>().ToConstant(new Mock<IMailServer>().Object);
After binding these two dependencies the test finally turns up green.

This development experience was horrible. The Servicelocator pattern fails at making it easy for me to discover dependencies, leading to an interrupted test flow. Another harmful side-effect is that it also becomes harder for me to read code and understand the high level interactions between objects, without getting knees deep into the implementation details.

By hiding your dependencies, you also promote ignorance to a problematic amount of dependencies. I discovered three in this example, but as this class grows, more and more of them will be buried inside the method implementations.

To make it easier to avoid and spot these smells earlier on, without dropping the Servicelocator pattern as a whole, you could move service resolution to the constructor. This isn't a completely safe refactoring though. It's possible that some instantiations are expensive, and should be implemented to be lazy initialized.

There is one more annoyance I would like to spout in this post. The Servicelocator pattern doesn't encourage good OO. I'm not an OO purist, but seeing dozens of static classes resolving their dependencies through a servicelocator sends shivers down my spine. I would rather pick an Inversion of Control technique which basically makes it impossible to do this.

This post turned out to be longer than I expected it to be, and I even left some annoyances uncovered. I hope I succeeded in explaining my beef with the Servicelocator pattern. I trust that the general consensus will soon dictate that this pattern easily leads to marginal code, and that it actually is an anti-pattern which should be avoided if possible.

What is your experience with the Servicelocator pattern?

Wednesday, April 11, 2012

Visualizing the offline application cache update progress

I wrote about using the HTML5 application cache earlier, mostly focusing on generating and serving the manifest file using ASP.NET MVC. I also bitched about how not one browser I know of gives an indication of the application cache update progress. Today I wanted to write something about how you can visualize the application cache update progress yourself.

The applicationCache API has several useful and rather straightforward events we can handle to inform the user of the update progress.
window.applicationCache.onchecking = function (e) {
    updateCacheStatus('Checking for a new version of the application.');
};
window.applicationCache.ondownloading = function (e) {
    updateCacheStatus('Downloading a new offline version of the application');
};
window.applicationCache.oncached = function (e) {
    updateCacheStatus('The application is available offline.');
};
window.applicationCache.onerror = function (e) {
    updateCacheStatus('Something went wrong while updating the offline version 
                        of the application. It will not be available offline.');
};
window.applicationCache.onupdateready = function (e) {
    window.applicationCache.swapCache();
    updateCacheStatus('The application was updated. Refresh for the changes to take place.');
};
window.applicationCache.onnoupdate = function (e) {
    updateCacheStatus('The application is also available offline.');
};
window.applicationCache.onobsolete = function (e) {
    updateCacheStatus('The application can not be updated, no manifest file was found.');
};
One event that is particularly helpful is the progress event. This event fires every time a resource is downloaded and contains three useful attributes we can use to display the download progress: lengthComputable, loaded and total. These attributes should be fairly self-descriptive, but here is the relevant snippet of the W3C specificiations.
For each cache host associated with an application cache in cache group, queue a post-load task to fire an event with the name progress, which does not bubble, which is cancelable, and which uses the ProgressEvent interface, at the ApplicationCache singleton of the cache host. The lengthComputable attribute must be set to true, the total attribute must be set to the number of files in file list, and the loaded attribute must be set to the number of number of files in file list that have been either downloaded or skipped so far. The default action of these events must be, if the user agent shows caching progress, the display of some sort of user interface indicating to the user that a file is being downloaded in preparation for updating the application. 
To have a text that updates the percentage of downloaded resources on every download, I came up with this.
window.applicationCache.onprogress = function (e) {               
    var message = 'Downloading offline resources.. ';

    if (e.lengthComputable) {
        updateCacheStatus(message + Math.round(e.loaded / e.total * 100) + '%');
    } else {
        updateCacheStatus(message);
    };
};
These attributes seem to be implemented in WebKit browsers, but not in Firefox. Firefox will fall back to the static message 'Downloading offline resources..'. Internet Explorer doesn't support the offline application cache as a whole.

I'm sure more creative souls have it in them to build a really elegant and visually pleasing progress indication using this technique. I'm curious to hear (or see!) how you would represent the update process.

Monday, April 2, 2012

Check for local file browsing with JavaScript

Because I do most of my research while commuting by train, I often pull entire websites offline using httrack. While browsing the jQuery Mobile documentation locally this morning, I stumbled upon following gem.


I was curious to see how they determine whether a page is browsed locally or not. Looking into the source, I was a bit dissapointed to find nothing but plain common sense. The trick is comparing the protocol of the current location with known local protocols.
if ( location.protocol.substr(0,4)  === 'file' ||
     location.protocol.substr(0,11) === '*-extension' ||
     location.protocol.substr(0,6)  === 'widget' ) {
    // Disable AJAX support etc
}
If you would want to use that check in multiple locations in your codebase, you might want to extend the location object with an isLocal function.
window.location.constructor.prototype.isLocal = function() { 
    return this.protocol.substr(0,4)  === 'file' || 
            this.protocol.substr(0,11) === '*-extension' || 
            this.protocol.substr(0,6)  === 'widget'; 
}
The function could be used like this.
if (window.location.isLocal()) {
    // Disable AJAX support etc
}

Sunday, April 1, 2012

Add images to a GitHub readme

Today I wanted to add some screenshots to a GitHub readme for the sake of documenting. While this wasn't particularly hard, I do had to iterate a few times before I got it right.

Hosting the images

You could simply add the images to your repository and reference them using the raw url's, but this isn't very efficient. Using this method, every request needs to go through GitHub's application layer. It's far better to make use of GitHub Pages, a feature purely designed to publish web content. I also like how you're not polluting the repository this way.

Referencing the images

I'm using the - oh so beautiful and simple - markdown format for most of my readme's. The syntax for embedding images looks like this.

![My image](username.github.com/repository/img/image.jpg)

I hope this post fills in the void I stumbled upon when googling for useful pointers earlier today.