Separation of Concerns by example: Part 2

Separation of concerns is one of the fundamental tenets of good object-oriented design.  Anyone can throw a bunch of code in a method and call it a day, but that's not the most maintainable approach.  So far, we've looked at:

• Separation of Concerns - how not to do it
• Separation of Concerns by example: Part 1 - Refactoring away from static class

In this part, I'd like to look at removing the dependency on HttpContext.  Here's what our classes look like thus far:

public class CustomerManager
{
    [DataObjectMethod(DataObjectMethodType.Select, false)]
    public static List<Customer> GetCustomers(int startRowIndex, int maximumRows)
    {
        var finder = new CustomerFinder();
        return new CustomerFinder().FindAllCustomers(startRowIndex, maximumRows);
    }
}

public class CustomerFinder
{
    public List<Customer> FindAllCustomers(int startRowIndex, int maximumRows)
    {
        List<Customer> customers = null;
        string key = "Customers_Customers_" + startRowIndex + "_" + maximumRows;

        if (HttpContext.Current.Cache[key] != null)
        {
            customers = (List<Customer>) HttpContext.Current.Cache[key];
        }
        else
        {
            customers =
                (
                    from
                        c in DataGateway.Context.Customers
                    orderby
                        c.CustomerID descending
                    select
                        c
                ).Skip(startRowIndex).Take(maximumRows).ToList();

            if ((customers != null) && (customers.Count > 0))
                HttpContext.Current.Cache.Insert(key, customers, null, DateTime.Now.AddDays(1), TimeSpan.Zero);
        }

        return customers;
    }
}

The CustomerManager class is our presentation-layer service, and is only a very thin wrapper on top of the domain and application services.  We want to push the important business logic down into the heart of the domain as much as possible, where it belongs.

The goal is to write a single passing test for the CustomerFinder class.  Right now, that isn't possible because of the dependencies on HttpContext and DataGateway.

For now, the immediate concern has to be HttpContext.  Having a dependency directly on Linq to SQL is bad, but at least the test can run without it.  It's not so simple to remove the HttpContext dependency, as we have quite a few choices on how to do so.

Designing the dependency

We know we're going to use constructor injection for this dependency.  What that means is that we'll use an interface to act as a facade, and the CustomerFinder will use that interface to do its work.

Under test, we'll pass a fake instance to test the indirect inputs and outputs to the test, while at runtime, the "real" instance will be used.

So the basic idea will be to hide the caching part behind an interface.  But what should we hide?  We have three options:

• Create an IHttpContext interface to hide HttpContext.Current
• Create an ICache interface to hide the HttpContext.Current.Cache property
• Create a specialized interface to hide the entire cache calls

Abstracting HttpContext.Current

This is a huge rabbit hole we don't want to go down.  HttpContext is a very large class with a ton of methods and properties.  We could create an IHttpContext interface that only has one property for Cache, but that has its own issues, too.

The Cache class is sealed, meaning we can't subclass it to create a fake instance.  So we'd have to create an ICache implementation anyway to get around this limitation.  And at this point, what is IHttpContext giving us other than a window to the Cache?  Nothing.

Some MVC frameworks opt to hide the entire HttpContext behind an interface or an abstract class.  These still violate the Interface Segregation Principle, as implementers of the interface have to provide implementations of Request, Response, Session, Cookies, etc etc.

This isn't a good choice, but what about a targeted Cache implementation?

Abstracting Cache

This looks like a better choice, as the Cache class is much smaller and lighter and HttpContext.  However, it still suffers from similar problems as the IHttpContext, where implementers have to know a great deal about hows and whys of Cache to get it to work properly.

One other issue an ICache implementation won't solve is the non-intention-revealing interface the Cache.Insert method contains. It's difficult to read the code inside CustomerFinder to see what the caching strategy is.

With that strategy out, we're down to our final option.

Specialized interface

Specialized interface entails creating an interface that only hides what we use, and providing use-specific names for the methods.  Instead of Cache.Insert, we might have Cache.InsertItemExpiringTomorrow.  It's a much more explicit interface, describing the intent of what our caching logic is doing.

Creating the specialized interface

As always, we'll first want to extract our methods out of the CustomerFinder class, providing good names for each of the methods.  We'll need to extract both the section that peeks inside Cache, retrieves an item from Cache, and inserts an item.  Along the way, we want to make sure we use the same names as what our interface will provide, which will save a rename step later.

After the Extract Method refactorings, our class now looks like this:

public class CustomerFinder
{
    public List<Customer> FindAllCustomers(int startRowIndex, int maximumRows)
    {
        List<Customer> customers = null;
        string key = "Customers_Customers_" + startRowIndex + "_" + maximumRows;

        if (Contains(key))
        {
            customers = GetItem(key);
        }
        else
        {
            customers =
                (
                    from
                        c in DataGateway.Context.Customers
                    orderby
                        c.CustomerID descending
                    select
                        c
                ).Skip(startRowIndex).Take(maximumRows).ToList();

            if ((customers != null) && (customers.Count > 0))
                AddItemExpringTomorrow(key, customers);
        }

        return customers;
    }

    private void AddItemExpringTomorrow(string key, List<Customer> customers)
    {
        HttpContext.Current.Cache.Insert(key, customers, null, DateTime.Now.AddDays(1), TimeSpan.Zero);
    }

    private List<Customer> GetItem(string key)
    {
        return (List<Customer>) HttpContext.Current.Cache[key];
    }

    private bool Contains(string key)
    {
        return HttpContext.Current.Cache[key] != null;
    }
}

But this isn't quite right, is it?  We have a few problems already:

• Contains is positive logic, while the operation performs negative logic.  The logic is reversed
• The Insert and Get methods are coupled to the List<Customer> type.  We're not making an ICustomersCache, so generics will help here.

After fixing the changes above, our extracted methods now look like this:

private void AddItemExpringTomorrow<T>(string key, T item)
{
    HttpContext.Current.Cache.Insert(key, item, null, DateTime.Now.AddDays(1), TimeSpan.Zero);
}

private T GetItem<T>(string key)
{
    return (T) HttpContext.Current.Cache[key];
}

private bool Contains(string key)
{
    return HttpContext.Current.Cache[key] == null;
}

That's much better.  Now that we have a group of methods with our Cache logic we need, we can perform Extract Class to create our specialized Cache implementation.  All we'll need to do is create a CustomCache class and move these methods to that class:

public class CustomCache
{
    public void AddItemExpringTomorrow<T>(string key, T item)
    {
        HttpContext.Current.Cache.Insert(key, item, null, DateTime.Now.AddDays(1), TimeSpan.Zero);
    }

    public T GetItem<T>(string key)
    {
        return (T)HttpContext.Current.Cache[key];
    }

    public bool Contains(string key)
    {
        return HttpContext.Current.Cache[key] == null;
    }
}

Our CustomerFinder class isn't compiling, but we have one more step for the CustomCache class before it's ready for prime time.  We need to Extract Interface so that our CustomerFinder implementation has something less concrete to work with.  ReSharper lets us extract the interface easily, which leaves us with our final CustomCache implementation:

public interface ICustomCache
{
    void AddItemExpringTomorrow<T>(string key, T item);
    T GetItem<T>(string key);
    bool Contains(string key);
}

public class CustomCache : ICustomCache
{
    public void AddItemExpringTomorrow<T>(string key, T item)
    {
        HttpContext.Current.Cache.Insert(key, item, null, DateTime.Now.AddDays(1), TimeSpan.Zero);
    }

    public T GetItem<T>(string key)
    {
        return (T)HttpContext.Current.Cache[key];
    }

    public bool Contains(string key)
    {
        return HttpContext.Current.Cache[key] == null;
    }
}

Now the ICustomCache is something that the CustomerFinder can deal with.  Since we're using constructor injection, we'll want to create a constructor that takes an ICustomCache implementation.  Our CustomerFinder will now look like:

public class CustomerFinder
{
    private readonly ICustomCache _customCache;

    public CustomerFinder(ICustomCache customCache)
    {
        _customCache = customCache;
    }

    public List<Customer> FindAllCustomers(int startRowIndex, int maximumRows)
    {
        List<Customer> customers = null;
        string key = "Customers_Customers_" + startRowIndex + "_" + maximumRows;

        if (! _customCache.Contains(key))
        {
            customers = _customCache.GetItem<List<Customer>>(key);
        }
        else
        {
            customers =
                (
                    from
                        c in DataGateway.Context.Customers
                    orderby
                        c.CustomerID descending
                    select
                        c
                ).Skip(startRowIndex).Take(maximumRows).ToList();

            if ((customers != null) && (customers.Count > 0))
                _customCache.AddItemExpringTomorrow(key, customers);
        }

        return customers;
    }

}

Our CustomerFinder now has no opaque dependency on the Cache or HttpContext classes.  We only deal with our wrapper, which has more explicit names about what we're trying to do.

But now our CustomerManager class isn't compiling, as we removed the no-argument constructor for CustomerFinder.  In this case, I'll just allow the CustomerManager to instantiate the correct implementations of ICustomCache:

public class CustomerManager
{
    [DataObjectMethod(DataObjectMethodType.Select, false)]
    public static List<Customer> GetCustomers(int startRowIndex, int maximumRows)
    {
        var finder = new CustomerFinder(new CustomCache());
        return finder.FindAllCustomers(startRowIndex, maximumRows);
    }
}

We could have gone several ways on that one, from a creation method, to a factory, all the way to an IoC container like Spring or StructureMap.  We'll wait on any changes like that.

Quick review

Our CustomerFinder class had an opaque dependency on HttpContext.Current and Cache.  We looked at a few options at making that dependency explicit through constructor injection, settling on a specialized implementation that contained intention-revealing names for only the operations we support.  To do so, we performed a number of refactorings:

• Extract Method
• Extract Class
• Extract Interface

Finally, we fixed the CustomerManager class to use our new constructor with the appropriate dependency.  The maintainability of the CustomerFinder class has improved significantly, as well as usability, as both users and maintainers of this class can see immediately the explicit dependencies this class requires.

Next time, we'll look at getting rid of that pesky Linq to SQL dependency.