Kill switch, we've got Action!

Kill switch already

Have you ever struggled just a tiny bit following the conditional logic in a switch statement? Ever made a bug with them? How 'bout that neat extra break statement you need to put in to prevent it from falling through. In the old days, we found falling through useful. Nowadays, we aren't even allowed by the compiler. Yet we persist in adding case and break as block delimiters in those long long cascades of code.

I've been reading a few new blogs today, and I stumbled over a few fairly ugly switches. So I decided to tell y'all what I think about them. I'll also show you a couple of alternatives I personally find easier on the eyes. Incidentally, they'll also lend themselves to more maintainable code, including opening doors to some nice architectural patterns.

These techniques are far from new, switch is a well recognized "code-smell". There's references at the bottom of the article.

Let's just dive into an example. Say we have some input and we want to vary the output based on what it is. It can be based on the state of any object or value we have, not just strings. However, I'll make it simple for the sake of the example.

We have some input, and we'll decide whether to kill switch:

void KillOrNot(string input)
{
    var output = "";
    var i = 0;

    switch(input)
    {
        case "kill-switch-now":
            output = "aaargh";
            break;
        case "long-live-switch":
            output = "noooooo";
            i = -1;
            break;
    }

    // do something with this information
}

Notice the individual logic with i in there. I don't quite get what i does, but I'm sure it's in there for a purpose. With i in there however, this code is as bad as it gets. Imagine it with yet another variable and 15 more cases. If you really, really like such switches and want to stop reading, I recommend you at least put those blocks of logic into each of their own functions. That way each block will tell you it's purpose. We'd put the kill part in a Kill() method, and the live part in a LiveAndDecrease() method. At least it would cut down the size, somewhat, and we'd know what it does. We'd have one line of code per case. (Variables would be promoted to members.)

But let's put the i away for a moment. It probably violates the single responsibility principle anyway. I'll get to the really good part later. Now, to swap one string for another, there's a neat class we can use for pairs of strings instead. It'll be shorter and more concise. It's called a Dictionary. (Or it's sometimes faster siblings.) Here's the string lookup refactored to use a dictionary:

void Now(string input)
{
    var outputs = new Dictionary<string, string>
    {
        { "kill-switch-now", "aaargh" }
        { "long-live-switch", "nooooo" }
    };

    var output = outputs[input];

    // do something with this information
}

Notice how the actual result gets as close to the predicate as possible. It's a lot easier to follow. Imagine this with 20 cases instead of 2.

With simple strings out of the way, how how about that i. I'll cross my fingers and hope you agree it's easier to work with if each case has it's own method. So we've promoted output and i, and we've created these two Kill and LiveAndDecrease methods:

string output;
int i;

void Kill()
{
    output = "aaaargh";
}

void LiveAndDecrease()
{
    output = "nooooo";
    i = -1;
}

Did you guess what's next? It's another dictionary. But we'll swap out the of-type part. We'll use Action:

Dictionary<string, string> handlers = new Dictionary<string, Action>
{
    { "kill-switch-now", Kill }
    { "long-live-switch", LiveAndDecrease }
};

And to invoke it, we just call the dictionary item:

void Now(string input)
{
    handlers[input]();
}

How's that for readability? The only thing I have an issue with here is the invocation itself. However, it's one line, and we should probably name Now something meaningful for our context. Once you know this trick it stops being so arcane too.

If you have ReSharper installed, maintenance becomes a breeze. Whenever you write a new item in the dictionary, you'll get a small lamp on the nonexistent method name. Press Alt+Enter, and hey presto - there's a new method with the right signature!

If we need parameters for each block, we could either set some members, or we could add them as parameters to the action. A string parameter would be Action<string>, and the signature would be void Name(string parameter). We can of course also return stuff. So we'd go Func<string, string> and handlers[input](parameter).

All this is actually just poor man's strategy pattern. The bigger your blocks in the switch statement, the bigger the chance you actually have polymorphic behavior that belongs in several subclasses of a common type. For simple logic, there's nothing wrong with the poor man's edition. The fact that we've got methods for each block means we can easily extract each method to it's own class when the time comes to go big.

If you got this far and find these ideas interesting, I highly recommend you check out these links:

There's a lot more out there. Just google Clean Code.
Happy refactoring! :)

Author

comments powered by Disqus