Separation of Concerns by example: Part 1

20 June, 2008. It was a Friday.

In the prelude to this series, I looked at a snippet of code that took the kitchen sink approach to concerns. Here’s what we started out with:

public class CustomerManager
{
    [DataObjectMethod(DataObjectMethodType.Select, false)]
    public static List<Customer> GetCustomers(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;
    }
}

Before we can even think about separating the concerns out, we already have the design working against us. I prefer a dependency inversion approach to separate concerns, as opposed to having the class or method instantiate them all at once.

When we have a static method, how can we give the dependencies to the class? I can think of some clever ways, but clever is rarely simple. I’d like to err on the side of simple.

Additionally, we don’t want our domain services mixed in with presentation concerns, such as the DataObjectMethod attribute. That is in place for the ObjectDataSource control, a presentation object.

So, first order of business, let’s make this a plain old class, instead of a static class.

Refactoring away from static methods

Now, presentation-specific classes are just fine. If our presentation layer needs static methods and some crazy attributes, good for them.

But I don’t want those concerns bleeding down into my ~~BLL~~ domain layer.

To fix this, first we’ll need to extract method and get all of the contents out of that method (Ctrl+Alt+M ftw):

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

    private static 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;
    }
}

Now that we have a private method, we can use Move Method to move this method to a whole new class. If this method used any other private (or public) methods, we’d have to deal with them one at a time. For now, we’ll have a new CustomerFinder application-level service class:

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;
    }
}

Now, we don’t have any tests in place (this is a full-blown legacy app), so we’re just focusing on dependency-breaking techniques. These techniques preserve existing behavior, which is what we need when we don’t have a safety net of unit tests in place.

The existing CustomerManager now needs to change, as it needs to use our new class. No matter, we’ll just instantiate a new CustomerFinder and use that:

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

Now our presentation layer is just a very thin layer on top of our application and domain layer. Which is good, as the presentation layer is the hardest layer to test (and the absolute slooooooowest).

Quick review

We want to change the CustomerManager, but it’s a presentation layer class that we still need to service our user interface. To get around this, we applied a series of refactorings to move the behavior to an application layer service, which we can now work against for future enhancements. These refactorings included:

We did these in a way such the existing behavior of CustomerManager remained unchanged, but its responsibilities were moved to the CustomerFinder service.

Next time, we’ll look at the CustomerFinder to see how we can remove some of the static dependencies (I think I saw a screencast somewhere on that one…)

← Adios, Netflix profiles

Separation of Concerns by example: Part 2 →