Legacy Code to Testable Code #8: Convert If-Else’s to Guard Blocks

I know, you want to check out the rest of the series. Here are the links:

The Legacy Code To Testable Code Series

Code simplification is one of the best things we can do for ourselves. If we understand the code, not only will we write simpler tests, we will make less mistakes.

Problem is, we’re now treading into dangerous territory. Up until now, refactoring operations were simple, and therefore less risky. We could trust our tools to do them (mostly).

Now we actually need to understand what we’re doing.

Since we’re talking about logic in testing, there’s a lot of if’s and else’s that are candidates for testing. If we reduce the complexity, we’ll have less and simpler cases to test. One of the methods to do this is by creating guard blocks.

Guard blocks are early exit conditions from the function. You’ve probably seen this structure of a method before:

public void DoSomething()
{
    if (!condition1())
        return;

    if (!condition2())
        return;

    NowDoThatSomething();
}

The different exit conditions are there to, well exit the method, and early These are usually validations. The rest, important beefy part of the method is what’s left after we’ve gone through all the validations.

If we use TDD, code takes this shape most of the time. It’s simple, and lends itself to incremental development. If it grows to be more complex, we can later do something with all the validation, like move them to a separate class. For now, let’s keep this structure.

What happens if we don’t use TDD? Chances are, we arrive at a much more complex code. Like this:

public int SafePositiveDivide(int a, int b)
{
    int result = 0;

    if (b == 0)
    {
        result = -1;
    }
    else
    {
        if (a<=0 && b<0)
        {
            result = -1;
        }
        else
        {
            result = a / b;
        }
    }
    return result;
}

And that’s on a good day. Code starts out simple, but it’s easy to patch it with different cases, and make it more complex. That’s how we get complex code.

This code is also probably full of dependencies and more internal cases and for loops. And other constructs to make it look more “professional”.

Let’s simplify it, shall we?

To get from here to a more simple structure, the first step is to get rid of result, and replace it with return statements.

public int SafePositiveDivide(int a, int b)
{
    if (b == 0)
    {
        return -1;
    }
    else
    {
        if (a <= 0 && b < 0)
        {
            return -1;
        }
        else
        {
            return a / b;
        }
    }
    return 0;
}

I know, that’s not what they taught you at school. But bare with me, because we have an incoming message from the IDE.

What’s that?

We don’t need the final return because it’s unreachable code?

Good.

public int SafePositiveDivide(int a, int b)
{
    if (b == 0)
    {
        return -1;
    }
    else
    {
        if (a <= 0 && b < 0)
        {
            return -1;
        }
        else
        {
            return a / b;
        }
    }
}

The cool thing about early exit, is that it makes else constructs redundant. Let’s get rid of the first one. Things are moving to the left!

public int SafePositiveDivide(int a, int b)
{
    if (b == 0)
    {
        return -1;
    }
    if (a <= 0 && b < 0)
    {
        return -1;
    }
    else
    {
        return a / b;
    }
}

Come to think of it, we don’t need that second else as well.

public int SafePositiveDivide(int a, int b)
{
    if (b == 0)
    {
        return -1;
    }
    if (a <= 0 && b < 0)
    {
        return -1;
    }

    return a / b;
}

Now the structure starts to look familiar. Why don’t we extract that validation logic into a function:

public int SafePositiveDivide(int a, int b)
{
    if (ValidateResult(a, b))
        return a / b;
    return -1;
}

private bool ValidateResult(int a, int b)
{
    if ((b == 0) || (a <= 0 && b < 0))
    {
        return false;
    }
    return true;
}

Now we have two options. We can test the public SafePositiveDivide, which is the obvious choice. Or, we can extract the private ValidateResult function, into a separate Validator class, which can be tested on its own.

With complex code, it’s easy to miss conditions (especially if the information is complex, and there are many of them). That’s where the refactoring risk lies. We have a couple of tactics there too.

The first is to write characterization tests before we start messing with the code, and make sure that things still work as we are moving stuff around. It requires a high level of understanding, and probably much effort.

We can use tools like ApprovalTests or TextTest to help identify changes from the original behavior. These test compare the output into a recorded template, and whenever we’ve strayed from the path, the tool will tell us.

The final option, is what we intended to do originally – write unit tests. These tests should cover what we think the behavior should be. Note the use of the word “should”. We can only check what we can think of, not the other cases.

It is risky, but worth it. To mitigate the risk, add more eyes. Work in pairs (or mobs) to make sure things don’t go amiss.