Code coverage reports are neat. At a glimpse you can determine what pieces of code have been executed in a batch of unit tests, and get a good guesstimate on how well your unit testing efforts are going. Unfortunately there is a tendency to jump to the conclusion that covered code is tested code. If only it was that simple!

Assume we have a simple method:

function calculateTotal(amount, includeTaxes) {
    if (amount < 0) {
        raiseError();
    }
    var total = amount;

    if (includeTaxes) {
        total *= 1.05;
    }

    return total;
}

This can be tested fully with a few simple asserts

assertEquals(100, calculateTotal(100, false));
assertEquals(105, calculateTotal(100, true));
assertEquals(0, calculateTotal(0, false));
assertEquals(0, calculateTotal(0, true));
assertError(calculateTotal(-5. true));
assertError(calculateTotal(-5. false));

Running these tests will provide a coverage report that indicates we have 100% code coverage. That is good, since any fully tested function will have 100% code coverage, and our tool would be broken if it didn't say that. However, what happens if we need to add a small bit of functionality to this function? Maybe we want to include the option to include a default tip. Our function could be changed like this:

function calculateTotal(amount, includeTaxes, includeTip) {
    if (amount < 0) {
        raiseError();
    }
    var total = amount;

    if (includeTip) {
        total *= 1.15;
    }

    if (includeTaxes) {
        total *= 1.05;
    }

    return total;
}

Since we have added to the function we need to modify our tests or the compiler will ball at us. There also needs to be additional testing included to cover the new functionality. Ideally we would have done this TDD, but that's a topic for another day.

//existing tests modified for third param
assertEquals(100, calculateTotal(100, false, false));
assertEquals(105, calculateTotal(100, true, false));
assertEquals(0, calculateTotal(0, false, false));
assertEquals(0, calculateTotal(0, true, false));
assertError(calculateTotal(-5. true, false));
assertError(calculateTotal(-5. false, false));

//new test
assertError(115, calculateTotal(100, false, true));

If we run our tests again we will find that we have 100% code coverage again. It is tempting to assume that it is 100% tested, but jumping to the conclusion is wrong since we did not test how the function behaves if we are calculating both the tip and the taxes. Never assume that you have 100% test coverage if you have 100% code coverage.

Does this mean that the code coverage report is useless? I don't think so. While you cannot determine if a covered line is tested, you can determine that an uncovered line is not. This may give you a clue on the best place to start your unit testing effort.

In an earlier post I tried to outline the various reasons that I have heard for using a Single Exit Philosophy. I prefer not limiting myself to it.

One of the reasons that I mentioned before as an advantage for single exit functions was that it makes debugging easier. I thought I knew the reason for this but I cannot recall it. With good use of stack dumps and break points I cannot think of any reason why debugging would be easier.

The main reason why I like multiple exits is that it reduces the amount of nesting required. This makes any given function more readable since you do not have to immerse yourself extra if layers. Additionally it simplifies the logic since you do not have to carry a value to the end of your function if you can determine it earlier on.

For example, lets look at a piece of code that calculates the tax rate of an amount given the region. We are given the stipulation that it must return 0 as fast as it can for amounts of 0 or unknown regions since look ups (especially of unknown regions) are expensive. It should also return a negative number when it encounters an error.

function calculateTaxes(amount, region) {
    var taxes;
    if (region != null and amount > 0) {
        var lookupFailure = false;
        var nationalTax;
        var provincialTax;

        nationalTax = lookupNationalTax(region);
        if (nationalTax < 0) {
            lookupFailure = true;
        }

        if (!lookupFailure) {
            provincialTax = lookupProvincialTax(region);
            if (provincialTax < 0) {
                lookupFailure = true;
            }
        }

        if (!lookupFailure) {
            taxes = amount * (nationalTax + provincialTax);
        } else {
            taxes = -1;
        }
    } else {
        taxes = 0;
    }
    return taxes;
}

Refactoring this to use multiple returns gains us several advantages:

• Fail fast code appears near the top of the function
• Removes nesting
• Removes variables

Would become:

function calculateTaxes(amount, region) {
    if (region == null or amount == 0) {
        // fast fail. looking up taxes is expensive
        return 0;
    }

    var nationalTax = lookupNationalTax(region);
    if (nationalTax < 0) {
        return -1;
    }

    var provincialTax = lookupProvincialTax(region);
    if (provincialTax < 0) {
        return -1;
    }

    return amount * (nationalTax + provincialTax);
}

I’m not sure about you, but I would be much happier maintaining the second piece of code. It is easier to follow and shorter to boot.