Notes from my Brown Bag Learning Forum Presentation. Download the source code, or just sit back and relax.

GoF Patterns:

• Chain of Responsibility
• Decorator
• Adapter

Buzz Phrases:

• Single Responsibility Principle
• Open-Closed Principle
• Inversion of Control
• Aspect Oriented Programming

Embracing the Single Responsibility Principle, Open-Closed Principle and Inversion of Control results in trading fragile application logic for fragile configuration logic. That’s a pretty good trade.

Fragile application logic is costly and it will come back to hurt you repeatedly. It goes without saying that fragile application logic is not testable, otherwise it wouldn’t be fragile. No tests mean changes are scary, so you have to compensate by regaining an intimate understanding of all the twists and turns in order to have enough confidence to make the change. The time and mental energy it takes to work through delicate and subtle conditional logic is enormous. My mediocre brain can only manage a short call stack and juggle a handful of variables at once.

Let’s say you’re sold on the promise of pretentious acronyms like SRP, OCP, IoC and the like. So now you end up with a billion small classes and gobs of code dedicated to wiring-up your favorite inversion of control container (mine is Unity). Are we better for it? Let’s examine this trade-off by implementing the same functionality conventionally and using fancy patterns and principles.

The Scenario: Implement captcha as the first step in some business process, like a registration form.

That’s pretty easy, I don’t need anything fancy to do that.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35

public class SimpleController : Controller {
    private const string CaptchaTextKey = "captcha_text";

    public ActionResult Index() {
        return View();
    }

    public ActionResult Render() {

        var captchaImage = new CaptchaImage();

        HttpContext.Session[CaptchaTextKey] = captchaImage.Text;

        using (Bitmap bitmap = captchaImage.RenderImage()) {
            using (var stream = new MemoryStream()) {
                bitmap.Save(stream, ImageFormat.Jpeg);
                return new FileContentResult(stream.ToArray(), "image/jpeg");
            }
        }
    }

    [AcceptVerbs(HttpVerbs.Post)]
    public ActionResult Verify(string captchaText) {

        var actualText = (string)HttpContext.Session[CaptchaTextKey];

        if (captchaText.Equals(actualText)) {
            ViewData["message"] = "You are a human";
        } else {
            ViewData["message"] = "fail";
        }

        return View("Index");
    }
}

Fast forward 3 months and amazingly new requirements have crept into the simple captcha controller. Consider these contrived yet poignant scenarios:

• After the project goes to QA, you realize there needs to be a way to bypass the captcha check so automated Selenium tests can get to the rest of the application.
• After the project goes live, the business decides it wants to know how many users are hitting the form. So an audit log is added.
• After reviewing the audit log, it is discovered that some IPs are attacking the form, so we decide to implement a black list.
• After the black list is live, the servers begin to perform slowly, so detailed logging is added.
• The logs show the black list lookup is slow during attacks, so it is determined that caching should be implemented.
• The business is doing a television promotion and expects traffic spikes. IT wants real-time visibility to monitor the application during heavy load.

Now our SimpleController has morphed into a SmellyController:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61

public class SmellyController : Controller {

    // ...

    [AcceptVerbs(HttpVerbs.Post)]
    public ActionResult Verify(string captchaText) {

        var ip = HttpContext.Request.ServerVariables["REMOTE_ADDR"];

        var isBlackListed = IsBlackListed(ip);

        if (IsMatch(captchaText) && !isBlackListed || captchaText == "selenium" {
            ViewData["message"] = "You are a human";

            // reset consecutive error count
            ConsecutiveErrorCount = 0;
        } else {
            ViewData["message"] = "fail";

            // add to black list
            if (ConsecutiveErrorCount++ >= 3 && !isBlackListed) {
                AddToBlackList(ip);
            }
        }

        WriteToAuditLog(ip);

        return View("Index");
    }

    private bool IsBlackListed(string ip) {
        var sessionKey = BlackListKey + ip;
        object result = HttpContext.Cache[sessionKey];

        if (result == null) {
            using (var connection = new SqlConnection(connectionString)) {
                connection.Open();
                var sql = "select count(*) from blacklist where ip = @ip";
                using (var command = new SqlCommand(sql)) {
                    command.Connection = connection;
                    command.Parameters.AddWithValue("@ip", ip);
                    result = Convert.ToBoolean(command.ExecuteScalar());
                }
            }
            HttpContext.Cache[sessionKey] = result;
        }

        return (bool)result;
    }

    private int ConsecutiveErrorCount {
        get { return Convert.ToInt32(HttpContext.Session[ErrorCountKey] ?? "0"); }
        set { HttpContext.Session[ErrorCountKey] = value; }
    }

    // ...

    private bool IsMatch(IEquatable<string> captchaText) {
        var actualText = (string)HttpContext.Session[CaptchaTextKey];
        return captchaText.Equals(actualText);
    }

How does this smell? Let me count the ways:

1. Hard to test
2. Mixed levels of abstraction
3. No separation of concerns

What if we had followed Uncle Bob’s SOLID principles? Our controller might look like this:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30

public class SolidController : Controller {
    private readonly ICaptchaProvider captchaProvider;

    public SolidController(ICaptchaProvider captchaProvider) {
        this.captchaProvider = captchaProvider;
    }

    public ActionResult Index() {
        return View();
    }

    public ActionResult Render() {
        using (var stream = new MemoryStream()) {
            captchaProvider.Render(stream, ImageFormat.Jpeg);
            return new FileContentResult(stream.ToArray(), "image/jpeg");
        }
    }

    [AcceptVerbs(HttpVerbs.Post)]
    public ActionResult Verify(string captchaText) {

        if (captchaProvider.Verify(captchaText)) {
            ViewData["message"] = "You are a human";
        } else {
            ViewData["message"] = "fail";
        }

        return View("Index");
    }
}

And our original simple captcha provider could look like this:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18

[Serializable]
public class SimpleCaptchaProvider : ICaptchaProvider {
    private string captchaText;

    public void Render(Stream stream, ImageFormat format) {
        var captchaImage = new CaptchaImage();

        captchaText = captchaImage.Text;

        using (Bitmap bitmap = captchaImage.RenderImage()) {
            bitmap.Save(stream, format);
        }
    }

    public bool Verify(string text) {
        return text.Equals(captchaText);
    }
}

If you’re wondering why we don’t have to store the captcha text in the session, it’s because we’re putting the onus on the container to give us the same instance of SimpleCaptchaProvider each time it’s requested in the same session.

Let’s revisit the list of features that made our controller smelly and see how we could have done it open-closed style (by writing new code instead modifying old code). The go-to technique for this is the decorator pattern. So let’s make a decorator to look for a secret captcha password that our selenium test knows and let it through.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23

public class SeleniumBypassCaptchaProvider : ICaptchaProvider {
    private readonly ICaptchaProvider captchaProvider;
    private readonly string password;

    public SeleniumBypassCaptchaProvider(
        ICaptchaProvider captchaProvider,
        string password) {

        this.captchaProvider = captchaProvider;
        this.password = password;
    }

    public void Render(Stream stream, ImageFormat format) {
        captchaProvider.Render(stream, format);
    }

    public bool Verify(string captchaText) {
        if (captchaText == password) {
            return true;
        }
        return captchaProvider.Verify(captchaText);
    }
}

Next is the audit log, then the black list. These could be implemented as two more decorators, however we’re outgrowing this solution which means it’s time to refactor. Let’s switch hats for a few minutes and promote our decorator chain into an explicit chain of responsibility. This is like adding another lane to the freeway, it really opens things up. We’re modifying existing code so maybe you’re wondering what happened to our open-closed principle? It’s still there, I promise. The first point I’ll make is that refactoring is a special activity. It does not change the observable behavior of the application. When working with single responsibility classes, all we end up doing is adapting the logic to a different interface. In our case, we’re moving logic from BlackListCaptchaProvider to BlackListVerifyFilter. The logic stays intact and the unit tests are minimally impacted.

The end result of this refactor might look like this:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25

public class VerifyChainCaptchaProvider : ICaptchaProvider {
    private readonly ICaptchaProvider captchaProvider;
    private readonly IServiceProvider serviceProvider;

    public VerifyChainCaptchaProvider(
        ICaptchaProvider captchaProvider,
        IServiceProvider serviceProvider) {

        this.captchaProvider = captchaProvider;
        this.serviceProvider = serviceProvider;
    }

    public void Render(Stream stream, ImageFormat format) {
        captchaProvider.Render(stream, format);
    }

    public bool Verify(string captchaText) {
        return new Pipeline<string , bool>(serviceProvider)
            .Add<AuditLoggingFilter>()
            .Add<BlackListingFilter>()
            .Add<SeleniumBypassFilter>()
            .Add(new CaptchaProviderAdapter(captchaProvider))
            .Process(captchaText);
    }
}

Was it worth it? Well, we’re left with all these little classes each with their single responsibility, however we still have to wire it up. I’m not going to lie, it’s ugly and it’s fragile. So why is this design any better? It’s better because troubleshooting bad configuration is better than troubleshooting bad application logic. Bad application logic can do really bad things. In a 24/7 business-critical application, this usually happens around 3 AM and involves you waking up and trying adjust your eyes to a harsh laptop screen. With bad configuration on the other hand, whole chunks of functionality are just missing. Chances are your app won’t even start, or maybe it starts but the black list or the audit logging isn’t wired in. These issues are easy to test and when you fix them, you have enormous confidence that the functionality you just wired-in will work and continue to work in the wee hours of the morning.

The second point I’ll make is the code more closely follows the way we think about our application. This makes it easier to respond to change because new requirements are extensions of the way we already think about our application. Consider the scenario that our selenium secret passphrase is not secure enough for production and we want to add an IP restriction or signature to make sure it’s really our selenium test that is getting through. In our smelly controller, a selenium test bypass is not an explicit concept, it’s just an or-clause tacked on to the end of an already abused if statement. We’ll have to go into our smelly controller and do some thrashing around in the most important and delicate block of code. In our solid controller however, we have a nicely abstracted testable single responsibility class we can isolate our change to.

As another example, consider the scenario that our black list caching is consuming too much memory on the web server. With our SOLID design we can surgically replace our ICacheProvider with an implementation backed by Memcached. Bits of functionality are free to evolve at their own pace. Some areas of your application will need a beefy solution and some will be just fine with a simple one. The important thing is that concerns are isolated from each other and allowed to fulfill their own destiny.

Aspect-Oriented Programming

I mentioned aspect oriented programming at the beginning of the article in a shallow attempt to pique your interest. So before I wrap things up I’ll show how it fits in. Since we’re already using an IoC container and faithfully employing our SOLID design principles, we pretty much get AOP for free. This is a big deal. Software running under service level agreements and government regulations demands visibility and having aspects in your toolbox is a must. Because aspects are reusable, they are typically higher quality and more mature than something hand-rolled for a one-off scenario. And because they are bolt-on, our core business logic stays focused on our business domain.

Consider the cliché logging example. It’s overused, but works well not unlike the calculator example for unit testing or the singleton job interview question. The idea is that we tell our IoC container to apply a logging aspect to all objects it has registered. Here’s what my logging aspect produces:

DEBUG VerifyChainCaptchaProvider.Render() stream = MemoryStream, format = Jpeg
DEBUG SimpleCaptchaProvider.Render() stream = MemoryStream, format = Jpeg
DEBUG SimpleCaptchaProvider.Render() [72 ms]
DEBUG VerifyChainCaptchaProvider.Render() [74 ms]
DEBUG VerifyChainCaptchaProvider.Verify() text = afds
DEBUG AuditLoggingFilter.Process() input = afds
DEBUG BlackListingFilter.Process() input = afds
DEBUG CachingBlackListService.IsBlocked() ip = 127.0.0.1
DEBUG CachingBlackListService.IsBlocked() > False [0 ms]
DEBUG SeleniumBypassFilter.Process() input = afds
DEBUG SimpleCaptchaProvider.Verify() text = afds
DEBUG SimpleCaptchaProvider.Verify() > False [0 ms]
DEBUG SeleniumBypassFilter.Process() > False [1 ms]
DEBUG BlackListingFilter.Process() > False [4 ms]
DEBUG AuditLoggingFilter.Process() > False [8 ms]
DEBUG VerifyChainCaptchaProvider.Verify() > False [14 ms]

Again, this is powerful because we didn’t have to pollute our code with logging statements, yet we see quality log entries with input, output and execution time. In addition to logging, we can attach performance counters to meaningful events, add exception policies to notify us when things go wrong, selectively add caching at run-time and lots more. To see it all in action, you can download the source code for the examples used in this article.

The difference between Enterprise software and regular software is like the difference between a start-up and a public company. Enterprise software has lots of bureaucracy and red tape (indirection and configuration) to support transparency and top-down policy. You pay a price up front to create all these layers and well-defined communication channels between departments, but once the whole thing is up and running, it’s powerful.

Big companies can throw time and money at problems to produce activity diagrams, advisory boards and flow charts. Sure there’s bloat, but you need that kind of oversight to enforce corporate policy. Similarly, enterprise software can extravagantly use cpu cycles, disk space and memory, but this is a fair price to pay when complexity is king. Enterprise software must be concerned with things like service level agreements and government regulations. So how do you do that?

You do that by dissecting your application into composable units of behavior. This is the essence of the single-responsibility-principle and while the concept is deceptively simple, it can have a profound effect on your software. Yes, the end result is lots of little classes, but it’s much more than that. These classes capture individual business concepts and overtime your software becomes a domain-specific-language (DSL). I don’t mean the kind of DSL that a business person would use directly, but more like xpath and regular expressions except your DSL is focused on your business domain.

Over time, software becomes an asset or a liability. The difference is your ability to respond to the needs of the business and your toolbox for that is your DSL or lack thereof. You need to compose, decompose, rearrange and built-out bits of behavior in ways impossible to predict. Single responsibility gives you that ability and dependency inversion creates seams throughout your code to spin off in unpredictable directions. These seams are the key to longevity and the difference between enterprise software and regular software.

I’ll share an experience I had this weekend. I was about to deploy a new application and I thought it would be cool to use a performance counter to watch throughput. Within about 20 minutes I was able to add a performance counter by only modifying the App.config and this was the result:

This was possible because of IoC among other things. The app is composed of many small pieces and you’d be hard pressed to find the new keyword where it matters. SyncListingJob was already being built-up by Unity Container, so I was able take advantage of an off-the-shelf performance monitor aspect in the Enterprise Library 4.1 Policy Injection Application Block and wire it in via the App.config.

There is a convention programmers go by that says object names should be nouns and methods names should start with a verb. That’s crap and I’ll tell you why. First off, it’s an old rule kind of like Hungarian notation. Okay that was low. To be fair, I would probably recommend this rule to a college student.

However, if you’re doing this as your full-time job and you’re neck deep in a complex business domain, then you are seriously selling yourself short. When working in a team environment you’ve got to use every means possible to communicate what you were thinking and what hard-earned knowledge you gained along the way. Some poor sucker is going to open your project at some point and your code should be screaming important concepts right from the class list.

I don’t expect you to understand how the app works, but you should at least get a feel right away for the things that are important. Knowing it’s a console app, you could find the entry point and quickly navigate to the meat and potatoes of the application.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37

public class SyncListingJob : MarshalByRefObject, IJob
{
    private IServiceProvider locator;
    private YextListing listing;

    public SyncListingJob(IServiceProvider locator)
    {
        this.locator = locator;
    }

    public void Init(YextListing listing)
    {
        this.listing = listing;
    }

    [UnitOfWork]
    public void Execute()
    {
        GuardAgainstNotInitialized();
        SyncListing();
    }

    private void SyncListing()
    {
        new LocatorChain<Yextlisting>(locator)
            .AddNew<RevertDiscontinuedListing>()
            .AddNew<RefreshLinkedBusiness>()
            .AddNew<CreateLinkToExistingBusiness>()
            .AddNew<CreateNewBusinessAndLink>()
            .Process(listing);
    }

    private void GuardAgainstNotInitialized()
    {
        if (listing == null) throw new Exception("Job not initialized!");
    }
}

* The LocatorChain<T> is an implementation of the chain of responsibility pattern.

I do everything I can to push out all the noise and bring the behavior to the surface. Sometimes the right way to name a class is after a feature or behavior. I want my code to read like a DSL and making everything a noun just doesn’t cut it. The drivers for me are context and readability.

As pressure increases, testing decreases. As the testing decreases, errors increase and as errors increase, pressure increases more. This is what’s known as a positive feedback loop, and this was how I spent the better part of last week.

Figure A.5 Not enough time to test reduces the available time Test-Driven Development: By Example by Kent Beck

It didn’t start out that way. In fact the first few lines of code I wrote were tests. The problem was I could only get about an hour or two in each night and every day that passed we were leaving money on the table.

Since I didn’t have much time at night, I didn’t want to spend it writing tests. I just wanted to get the app done. All I needed to do was sync business listings from an affiliate with GarageCommerce.com, the project I’ve been working on for the last few months. There were only a handful of business rules to deal with and I thought I could just bang it out.

I know what you’re thinking. You already know where this is headed and you might as well just skip to the end. It’s like in that movie District 9. It started out pretty good with some edgy social commentary but by the end of the movie it just phoned in a cliché Hollywood plot. You know, like “fish out of water”, “buddy movie”, or “trading places” as was the case with District 9.

Anyway, I finished the app and since I only had a few tests I figured I should trace through things a few times to look for obvious problems. After doing that for a few minutes I was ready to press F5 and start working the kinks out. So after hours of zero feedback coding, I took a deep breath and the started the app for the first time.

It promptly bombed out as expected. I worked through a handful of errors and then it was good to go. As I watched it run, I thought to myself that maybe Uncle Bob is wrong about testing and Spolsky isn’t such a douche bag after all. However after processing a couple hundred jobs, it started throwing crazy errors in a tight loop. Crap!

Fast forward a few hours and I’ve got a hunch as to what is causing the problem. The way I wired up the app, I was creating a single NHibernate session for the entire batch of jobs. I knew this could be trouble so I at least made a point to commit after each job and clear the session so it wouldn’t get bogged down with 10K+ entities. I knew the single session was smelly and the errors I was seeing prompted me to refactor.

This is where it got ugly. I needed to do a pretty major overhaul and I was mostly test-less. My blood pressure went way up with each change and the fact that I didn’t have tests just made me take larger steps with no safety net. I felt like a M.A.S.H. surgeon with guts all over the operating table. Part of me thought I should take a few steps back and regroup, but another part of me was reminded of this quote:

If you’re going through hell, keep going. – Winston Churchill

So I stuck with it and got ‘er done, but it wasn’t much fun. What did I take from this experience? No revelations really, just some common sense reinforcement. It’s like rediscovering that diet and exercise is the key to losing weight, except that I rediscovered that not having tests really sucks when you have to make changes.

I like to think of the Main method in a Console application like the Global.asax. Its single responsibility is to wire-up things to be run in a console context. Each environment has unique configuration requirements. For example, when using NHiberate in a web application the lifetime of the ISession is attached to the HttpRequest, however in the context of a console application it may be a singleton or “per job” in a windows service. I like this:

1
2
3
4
5
6
7
8
9
10
11
12
13

class Program
{
    static void Main(string[] args)
    {
        using (var container = new UnityContainer())
        {
            container
                .AddExtension(new ConfigureForConsole(args))
                .Resolve<MyApplication>()
                .Execute();
        }
    }
}