Sunday, November 10, 2013

Get rid of switch statement

Yesterday I saw a piece of a legacy code that was written around 4 years ago. It is a plain old generic http handler that implements IHttpHandler interface. This handler is used for a simple data exchange between client code and a server through AJAX calls. This handler simply takes parameter sent from the client side and process them using some complex logic and switch statement. Let's take a look to a simple version of such handler. Let's imagine that our handler do some math operations:

public class Calculator : IHttpHandler
{

    public void ProcessRequest(HttpContext context)
    {
        context.Response.ContentType = "text/plain";

        var operation = context.Request.Params["operation"];

        var a = int.Parse(context.Request.Params["a"]);
        var b = int.Parse(context.Request.Params["b"]);

        var result = 0;

        switch (operation)
        {
            case "add":
                result = a + b;
                break;
            case "subtract":
                result = a - b;
                break;
            case "multiply":
                result = a * b;
                break;
            default:
                throw new NotSupportedException(string.Format("Operation {0} is not supported", operation));
        }

        context.Response.Write(result);
    }

    public bool IsReusable
    {
        get
        {
            return false;
        }
    }
}

It's pretty straight forward. The problem comes when we need to support a lot of operations. Then our switch statement becomes really big and heavy. How can we fix it!? Probably we need to do some refactoring. But first I'd extract all that logic to a separate class, so that our handler will take params pass them to the service and write a response back to a client. Another good reason to have a separate service is an ability to test our logic. So let's create a math service and call it from the handler.


public class MathService
{
    public int Perform(string operation, int a, int b)
    {
        switch (operation)
        {
            case "add":
                return Add(a, b);
            case "subtract":
                return Subtract(a, b);
            case "multiply":
                return Multiply(a, b);
            default:
                throw new NotSupportedException(string.Format("Operation {0} is not supported", operation));
        }
    }

    private int Add(int a, int b)
    {
        return a + b;
    }

    private int Subtract(int a, int b)
    {
        return a - b;
    }

    private static int Multiply(int a, int b)
    {
        return a * b;
    }
}

Now our handler looks like this:

public void ProcessRequest(HttpContext context)
{
    context.Response.ContentType = "text/plain";

    var operation = context.Request.Params["operation"];

    var a = int.Parse(context.Request.Params["a"]);
    var b = int.Parse(context.Request.Params["b"]);

    var mathService = new MathService();

    var result = mathService.Perform(operation, a, b);

    context.Response.Write(result);
}

Now our switch is inside MathService and it would be really cool to refactor it, but first we need to cover existing logic with unit tests in order to be sure that everything works as before after refactoring.
Let's write simple tests:


[TestFixture]
public class calculator_tests
{
    [TestCase("add", 3, 5, 8)]
    [TestCase("subtract", 3, 5, -2)]
    [TestCase("multiply", 3, 5, 15)]
    public void should_calculate_ints(string operation, int a, int b, int expectedResult)
    {
        var calculator = new MathService();

        int result = calculator.Perform(operation, a, b);

        Assert.That(result, Is.EqualTo(expectedResult));
    }
}

We have three operations in our service and three test cases. Now we are good to go.
What we can do here it's try to implement a design pattern called Match or Matcher. The point is that each our operation is a separate class and each class that presents math operation has a method called IsMatch so that we can peak up a right one from the list of all available operations. Let's switch to code.

public abstract class MathOperation
{
    private readonly string _operationName;

    public MathOperation(string operationName)
    {
        _operationName = operationName;
    }

    public bool IsMatch(string operationName)
    {
        return _operationName == operationName;
    }

    public abstract int Perform(int a, int b);
}

public class Add : MathOperation
{
    public Add() : base("add")
    {
    }

    public override int Perform(int a, int b)
    {
        return a + b;
    }
}

public class Subtract :  MathOperation
{
    public Subtract() : base("subtract")
    {
    }

    public override int Perform(int a, int b)
    {
        return a - b;
    }
}

public class Multiply : MathOperation
{
    public Multiply() : base("multiply")
    {
    }

    public override int Perform(int a, int b)
    {
        return a * b;
    }
}

We have a base abstract class MathOperation that has two methods: IsMatch and abstract method Perform. IsMatch checks if operations is appropriate, in our case we check using operation name, of cause in real situation we might have more complex logic. Now let's change our MathService.


public class MathService
{
    public int Perform(string operation, int a, int b)
    {
        var operationsList = new MathOperation[]
            {
                new Add(),
                new Subtract(),
                new Multiply()
            };

        var operationHandler = operationsList.Single(o => o.IsMatch(operation));

        return operationHandler.Perform(a, b);
    }
}

We defined a collection of the operations and using LINQ we take a single one that matches by operation name. Now let's run our test and we see that everything is OK. The good thing about it that we easily can add new operations and we can test each operation separately.