Separation of Concerns by example: Part 1

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…)

Related Articles:

Post Footer automatically generated by Add Post Footer Plugin for wordpress.

About Jimmy Bogard

I'm a technical architect with Headspring in Austin, TX. I focus on DDD, distributed systems, and any other acronym-centric design/architecture/methodology. I created AutoMapper and am a co-author of the ASP.NET MVC in Action books.
This entry was posted in Refactoring. Bookmark the permalink. Follow any comments here with the RSS feed for this post.
  • http://flimflan.com/blog Joshua Flanagan

    This is a great idea for a series, and I have a suggestion that might make it a little more effective.

    I assume the target audience is readers that are not yet used to this type of development, but are curious enough to learn. Chances are, they are not using ReSharper and references to things like Ctrl_Alt-M will be confusing. It may slow you down a bit, but I bet it would help the readers a lot if you suggest the default Visual Studio key mappings (Ctrl-R, Ctrl-M – I had to look it up) when available, or how to go about the refactoring manually if it is not available in VS.
    Yes, everyone should be using a tool like ReSharper, but lets not make it a barrier to entry to understanding the practices.

  • http://colinjack.blogspot.com/ Colin Jack

    As Joshua said its a great idea for a series, looking forward to reading the rest.

  • http://thebutlersspace.spaces.live.com/ Duncan

    Thanks, this looks like a great example, I am looking forward to the rest of the series, I like description of the refactoring to solve the static method problem.

  • http://www.eknacks.com/SingleListView.aspx?li=129 eKnacks

    You’ve been knacked. Keep up the good work.

  • http://wgyoypkasaxc.com/ pkehcjaeucf

    redUer vkenxctomraf, [url=http://svqpgdhekjrd.com/]svqpgdhekjrd[/url], [link=http://cxygayhghuwm.com/]cxygayhghuwm[/link], http://enpqieutrjek.com/