On the Merits of Simple Code

As developers, we seem to love building complex things. This might be because being good at complexity is a prerequisite to becoming a developer in the first place. But this doesn’t mean that complexity is always necessary.

In fact, I’d venture to suggest that we often make things more complicated than they need to be - perhaps because we don’t know any better, or perhaps because we stop as soon as we have something that works instead of finishing the job properly. Here’s an example of that last problem.

Problem Code

Imagine that you’re working on an application whose problem domain includes the following classes:

What comes to mind when you come across this code?

public int CountOfPartsOrdered(Customer myCustomer)
{
    var total = 0;
    var counts = new List<int>();
    for(int i = myCustomer.Orders.Count-1; i>=0; i--)
    {
        var order = myCustomer.Orders[i];
        foreach(var line in order.Lines)
        {
            var partCount = line.Quantity;
            total += partCount;
        }

        counts.Add(total);
        total = 0;
    }

    foreach(var c in counts)
    {
        total += c;
    }

    return total;
}

(Aside: apart from changing the problem domain, this code truthfully reproduces an actual fragment that I found at work this week.)

What observations do you have, what questions come to mind as you ponder this routine? Here are some that occurred to me …

• What is the list for?
• Why create a list of int ?
• Why iterate the list in reverse order using for()? It doesn’t seem to need it.
• Reusing a variable for two different purposes?

Simplified Code

I pretty quickly concluded that the last developer to touch this piece of code just stopped as soon as it worked, and never took the time to review and simplify.

With just a bit of inspection, it’s easy to simplify that method into this:

public int CountOfPartsOrdered(Customer myCustomer)
{
    return myCustomer.Orders.SelectMany(o => o.Lines).Sum(l => l.Quantity);
}

And yet, even this code is unsatisfactory - it seems overly complex, has a focus on the structure of other objects and violates the Law of Demeter.

Distributed Code

If we split the code out into smaller units and move them to the objects of interest, we end up with code that is much easier to understand.

Define the method CountOfPartsOrdered() on Order:

public class Order
{
    public int CountOfPartsOrdered()
    {
        return Lines.Sum(l => l.Quantity);
    }
}

Then we can define TotalCountOfPartsOrdered() on Customer:

public class Customer
{
    public int CountOfPartsOrdered()
    {
        return Orders.Sum(o => o.CountOfPartsOrdered());
    }
}

And our original routine becomes this:

public int CountOfPartsOrdered(Customer myCustomer)
{
    return myCustomer.CountOfPartsOrdered();
}

This last method is so simple that we might just end up writing the code inline instead of keeping it separate.

Analysis

The question becomes - are we any better off?

“Everything should be made as simple as possible, but not simpler.” – often attributed to Albert Einstein

I’d suggest we are, starting with the observation that our final solution eliminates a number of code smells (including feature envy) and provides methods on Customer and Order that will be useful elsewhere.

Further, there are metrics back up my assertion:

(Three lines are shown for the Distributed scenario, one for each of the final methods.)

• The original method, with a high Halstead Volume, required significantly more thought to understand.
• Overall maintainability has gone up as we made the code more declarative

What next?

My challenge to you is this: next time you think you’ve finished working with a piece of code, go back and give it one final look-over. This needn’t take more than a minute or two.

Is there anything you can do to simplify how it works? Is all your functionality in the right place? Are any of your classes acquiring too many responsibilities? Have you applied the Boy Scout Rule to everything you’ve touched? Have you made things easier or harder for the next developer? (Keeping in mind that the next developer might be you.)

Don’t settle for complex code where it’s not required. Relentlessly simplify. Remove complexity where you can, encapsulate it where you can’t. Make things better.