Is Your Code Hard to Understand?

Editorial Note: I originally wrote this post for the Infragistics blog.

I’ve heard a bit of conventional wisdom in the software industry.  It holds that people will read a given line of code an order of magnitude more times than they’ll modify it.  Whether that wisdom results from a rigorous study or not, I do not know. But it certainly squares with my experience.  And it also seems to square with the experience of just about everyone else.

Accepting this wisdom as axiomatic, code readability becomes an important business concern.  If you optimize for ease of writing at the cost of ease of reading, your business will lose money.  Better to spend some extra time in writing your code to ensure that future readers have an easy go of it.  And easy for them means that they understand the code.

You know of obvious ways to promote reader understanding in code.  Don’t give variables cryptic names or names that cannot be pronounced.  Don’t write gigantic methods and classes.  Limit method parameters and local declarations.  Read through the code out loud to see if it seems clear.  These guidelines will bring you a long way toward readability.

But other, subtle concerns can also chip away at your code’s readability.  I’ll list some of these here, today.  These are generally C# specific, but some are more broadly applicable than that.  What all of them have in common is that they constitute sources of confusion for readers that may not seem immediately obvious.

Magic Numbers

A magic number in your code is a numeric literal that exists without any explanation or hint as to its meaning.  These can feel like the most natural things in the world to write.  But they stymie maintenance programmers searching for meaning in the code.  Consider the following snippet.

public void ProcessInvoices(IList<Invoice> invoices)
{
    if(invoices.Count > 7)
    {
        foreach(var invoice in invoices)
        {
            invoice.Tabulate();
            _fileWriter.Write($"Invoice total is {invoice.Total} dollars.");
        }
    }
}

This method doesn’t present any obvious comprehension problems, with one glaring exception.  Cycle through all of the invoices, having them do internal tabulation and then writing the total out to a file.  And, do all of that if there are at least 7 invoices.  What, what?  Why 7?  Why is that particular number significant?

For a maintenance programmer, magic numbers render inscrutable the motivations for decisions made in code.  To make matters worse, the rationale for such decisions may get lost over time, leaving everyone to guess what, if anything, was so special about 7.  When you find yourself writing a magic number, spend a minute and declare a constant or a variable with a name like, “ProcessingBatchMinimum.”

Casting

While it may be controversial, one can certainly argue that casting is a code smell.  As former C# language team member Eric Lippert pointed out in this StackOverflow question, casting can indicate that you’re writing code where you have special knowledge that the compiler lacks.  And I don’t recommend pitting your C# knowledge against that of the C# compiler.

But, beyond possible maintenance and runtime correctness issues, casting adds a barrier to understanding.  The operation makes no conceptual, domain sense.  Rather, it represents language minutiae.  “Take this thing, turn it into this other thing for some reason, and then, if that works, do what matters to the new thing.”  That doesn’t exactly roll off the tongue in explanation.  Avoid casting as much as you can.

Lengthy Linq Chains

Linq gives C# programmers a great deal of power — power to create terse, functional-style expressions.  Linq brings a declarative paradigm to C# code, allowing developers to move away from comparably verbose imperative programming.  I strongly encourage you to take advantage of that.

But, at the same time, chaining Linq extension method calls together can lead to some pretty dense, hard to read code.  Have you seen declarations like this?

var distinctCoordinatesBelow = _window.GetAll<T>().Where(c => c.Location.Y > LabelYCoordinate).OrderBy(c => c.Location.Y).Select(c => c.Location.Y).Distinct();

Okay, let’s see.  Get all of something from the window, where the Y location is greater than something called “LabelYCoordinate.”  Then, order it by y coordinate.  Then, select Y coordinate.  Then, keep the ones that are distinct.

When you unpack this and read it aloud, you find yourself making several different statements.  Squashing them all into a single, dense line does not change that fact.  Linq is powerful, but I encourage you to break these sort of statements up using descriptive intermediate variable names.

Nested Conditionals

Have you ever read through code that contained an if within an else within another if, inside of a case statement?  Did you find that you needed to start jotting things down on paper to keep track of what was going on?  If so, rest assured that the fault lay with the author and not with your lack of understanding.

The human brain can store only so many “chunks” of information at one time, making it possible for you to remember a 7 digit phone number, perhaps recall a 10 digit one, but struggle with much beyond that.  That free context space fills quickly.

When you write code with nested if conditionals, you ask readers to fill that space remembering how they go to the current line.  “Oh, right, if x was equal to 7, but there are less than 15 entries in the list, and there are the global variable Y is equal to 4, and the op code Z is ‘ABC’, then…. wait, what where was I?”  Keep that to a minimum by keeping the number of nested conditionals to a minimum. Try not to ask your reader to keep track of more than 1 reason for being in a particular context within a method.

The Ternary Operator

You probably know what I mean when I say “ternary operator,” but, in case you don’t, consider the following example code.

public void ProcessInvoices(IList<Invoice> invoices)
{
    int x = invoices.Count > 7 ? invoices.Sum(i => i.Total) : 0;
}

The ternary operator involves the ? and the : and it gets its name from the fact that it takes 3 arguments: a boolean conditional, a value if true, and a value if false.  So, reading this as a story, we have “Set x equal to, if there are more than 7 invoices, the sum of the totals of the invoices, otherwise, 0.”  Another example that fails to roll nicely off of the tongue.

The ternary operator offers the handy ability to compact code and save yourself a verbose conditional.  Without it here, you would need to declare x, set it equal to 0, then update it to be equal to the sum of the invoice totals if the invoice count was greater than 7.  But in its own way, that verbose construct would present a more readable situation for the reader.

The ternary operator’s organization categorically eliminates it from reading left to right like common English in the way that many other statements do.  I am not advocating that you avoid it altogether, but I do suggest you bend over backward to make it obvious.  Extract the conditional into a method and add an extension method for the invoices to give you something that eliminates all noise.

public void ProcessInvoices(IList<Invoice> invoices)
{
    int x = ThereAreEnough(invoices) ? invoices.Total() : 0;
}

It still fails to read like conversational English, but now all that remains is single, clear arguments to the ternary operator.  Set x equal to, enough invoices, then total, otherwise 0.  You avoid the distraction presented by a bunch of other operators sprinkled among the ternary’s two.

All Food for Thought

I mentioned this when talking about the ternary operator, but it bears expanding to the whole conversation.  I am not trying to tell you exactly how to code, nor am I suggesting that your code is somehow irretrievably bad if it contains these things.  I am, however, suggesting that you keep an eye out for these concerns and understand the subtle drag effect they have on your code’s readability.  Because in a world where code is read so frequently, every little bit of readability counts.