Anti-Pattern 4: Using Global State

Let’s imagine we want to add caching capabilities for our InvoiceRepository class.

 

public class InvoiceRepository
{
    private IDataLayer _db;
    public InvoiceRepository(IDataLayer db)
    {
        _db = db;
    }
    public Invoice GetInvoice(int clientID)
    {
        Cache cache = Cache.GetInstance();
        string key = "Invoice" + clientID.ToString();
        if (cache[key]==null)
        {
            MeteringValues[] dailyValues = _db.GetMeteringValues(clientID);
            int offPeakPrice = _db.GetOffPeakPrice();
            int peakPrice = _db.GetPeakPrice();
            int advances = _db.GetAdvances(clientID);
            cache.Add(key, new Invoice(dailyValues, offPeakPrice, peakPrice, advances));
        }
        return (Invoice)cache[key];
    }
}

In this example we reach into the global state of our InvoiceRepository class and get a hold of the Cache singleton (Cache.GetInstance()). Global variables often show up as instances of the singleton pattern or just as static data in classes.

Not all singletons are bad design but all of them are suspect, presumed guilty until proven innocent! Nevertheless singletons which do not affect the functional behaviour of an application, can behave well as internal dependencies. A good example is the use of the singleton pattern for caching. Using the singleton pattern to implement caching is a valid strategy. The example here above illustrate this strategy nevertheless this example has a big flaw! The flaw is that we can’t intercept the call Cache.GetInstance. We can’t use a mocking framework to replace the Cache.GetInstance as this is a static method and static methods can’t be mocked (with Rhino Mock).

To solve this issue we need to extract the global state out of our Domain Object. We could for example pass a “Cache” object into the constructor:

public InvoiceRepository(IDataLayer db, ICache cache)

But imagine what would happen if we also want to log our exceptions and operations this would result in the following signature:

public InvoiceRepository(IDataLayer db, ICache cache, ILogger log)

 

When using the dependancy injection pattern (through constructor injection) every dependency to a service will result in a parameter we need to pass in our constructor.  That's why when we have a lot of dependencies (and usually we do)  using denpendancy injection result in classes that are difficult to instantiate because we need to inject all  cross cutting concerns (the cache, the logger, …) via the constructor. To solve this issue we can use the factory pattern. Nevertheless the Factory itself is still in his own way making our code more complex. This is why most of Unit test addicts advocates the use of IOC containers like Structuremap or Unity

Using an IOC container our code would look like this:

public class InvoiceRepository
{
    private IDataLayer _db;
    private ICache _cache;
    public InvoiceRepository()
    {
        _db = ObjectFactory.GetInstance<IDataLayer>();;
        _cache = ObjectFactory.GetInstance<ICache>();
    }
}

To test our SUT we can easily configure our container during the Arrange part of our unit test and don’t need to pass these dependencies through the constructor:

//Arrange
myIOCContainer.ForRequestedType<ICache>.TheDefaultIs<StubedCache>();


kick it on DotNetKicks.com