Is creating readable code an important feature of writing quality software?
Is it something that you would mention when reviewing code for one of your team members?
Typically, when we review code we focus on making sure that:
It does what it is supposed to do
It handles potential failures
It avoids duplication
It performs well if it is computationally intensive
But when it comes to the readability of someone's code, we often don't want to mention it.
Why?
For some, it might be related to fear of being accused of being petty or nit-picky. Others might not want to stifle a fellow developer's personal expression or creativity. And others might be more focused on getting things done and feel that focusing on readability is a waste of valuable developer time.
Is Readability Important?
Today, I would like to suggest that readability is important. And, I will start by providing a couple of code samples to demonstrate my point.
Take a look at the following SQL statement and think about how long it takes you to figure out what it does.
SELECT * FROM dbo.MyTable WHERE ((PATINDEX('%[0-9]%', @Filter) = 0) AND (@Filter LIKE '%,%'))
Now, take a look at the same code that has been made more readable.
SELECT *
FROM dbo.MyTable
WHERE (
(@filterContainsNumbers = 1)
AND
(@filterContainsComma = 1)
)
By using the simple concepts of using reader-friendly layout and using intention revealing variable names, someone can quickly scan your code and understand it within a matter of seconds. The first example would probably take a few minutes to restructure the layout and look up the definition of the PATINDEX function.
Take a look at another code example written in C#.
public void RenderRemainingMoop(Liability source)
{
var remainingMoop = source.OopRemaining + source.OopApplied;
return remainingMoop == 9999999999 ? String.Empty : OopRemaining.ToString("{0:c}");
}
Now take a look at the more readable version.
public void RenderRemainingMaxOutOfPocket(Liability liability)
{
const LiabilityMaxNumber = 9999999999;
var remainingMaxOutOfPocket = liability.OutOfPocketRemaining + liability.OutOfPocketApplied;
return remainingMaxOutOfPocket == LiabilityMaxNumber
? String.Empty
: OutOfPocketRemaining.ToString("{0:c}");
}
In addition to layout, using variables to provide context for magic numbers and strings and being descriptive instead of using abbreviations can better reveal what code is doing.
The Golden Rule
In both of the readable code samples, the developer focused not only on getting the job done, but also on making sure that future readers of their code would easily comprehend it.
And why is this important?
Because it makes the code more maintainable and flexible.
As mentioned in a previous post on software quality, maintainability is the effort required to locate and fix an error in an operational program. If your code is more readable, developers will be able to quickly understand it, which translates into quicker fixes. In that same article, it was also mentioned that flexibility is the effort required to modify an operational program. When developers need to add functionality to existing code, making it more readable will also allow them to quickly get up to speed and know where to place their functionality.
In both situations, you will be making someone else's job easier. As the golden rule states,
Do unto others what you would have them do unto you.
You never know when that someone else might be you!
Practicing the Golden Rule
So, what can we do to make our code more readable for others?
Using Intention Revealing Names
Everything in software has a name. And every time you name something, you have the opportunity to reveal its intention.
Be Descriptive
When naming things, be as descriptive as possible, and don't be afraid to use long names. Modern languages don't have many constraints when it comes to naming things, and the more descriptive, the better for the reader. There is no excuse for using abbreviations.
Provide Context for Magic Numbers and Strings
When using magic numbers or strings, make sure to assign them to a constant with a meaningful name. It helps to dispel the magic.
Be Ubiquitous
In Eric Evan's ground-breaking book, Domain-Driven Design: Tackling Complexity in the Heart of Software
, he establishes the concept of ubiquitous language, which establishes a common set of terms that can be used by both developers and business people alike. This helps to reduce the cognitive friction and potential confusion of having to translate between business concepts and code.
When you are naming something, prefer to use your domain's ubiquitous language rather than technical jargon. It will help reduce time and misunderstandings in the future.
Avoid Data Types in Names
In the past, using Hungarian notation to prepend an abbreviated data type to a variable name (examples: iCounter, bIsValid, txtFirstName, etc.) was in vogue, but it obscures the name by mixing the semantic meaning of a variable and its underlying data type. Modern development environments can easily reveal the type of a variable by hovering over the variable during debugging.
Using Reader-Friendly Layout
As shown earlier, laying out your code can make a big difference in how quickly a fellow developer (or even you, if you haven't seen the code for a while) can comprehend your code.
Show Logical Structure
Layout your code to emphasize the logical structure by using indentations and new lines. The examples below demonstrate this transformation.
--BEFORE
IF (@filter LIKE '%to%') BEGIN SELECT * FROM dbo.MyTable END
ELSE BEGIN SELECT * FROM dbo.MyOtherTable END
--AFTER
IF (@filter LIKE '%to%')
BEGIN
SELECT *
FROM dbo.MyTable
END
ELSE
BEGIN
SELECT *
FROM dbo.MyOtherTable
END
Avoid Run-on Sentences
Just as in good writing, you want to avoid the use of run-on sentences in your code. Instead of putting everything on one line as in the following example that uses a fluent syntax:
//BAD
var customers = eventsRepository.GetAll().Where(e => e.Date > DateTime.Now).Select(e => new Customer(e.FirstName, e.LastName));
you can do the following:
//BETTER
var customers = eventsRepository
.GetAll()
.Where(e => e.Date > DateTime.Now)
.Select(e => new Customer(e.FirstName, e.LastName));
Or, instead of nesting multiple statements into one line for efficiency:
//BAD
var customer = GetOrderFor(GetCustomerFor(HttpContext.Current.User.Identity.Name), Convert.ToInt32(Session["orderId"]));
you might want to do the following:
//BETTER
var username = HttpContext.Current.User.Identity.Name;
var customer = GetCustomerFor(username);
var orderId = Convert.ToInt32(Session["orderId"]);
var order = GetOrderFor(customer, orderId);
While there may not always be universally accepted practices when it comes to layout, each team should decide on rules for code formatting and stick to them. Static code analysis tools can help to enforce those rules and modern editors include utilities for automatically formatting code according to your rules as it is being written.
Handling Conditionals
Another area where code can become unreadable is when using boolean logic in if and while statements.
Extracting Conditional Logic
For example, while the following code sample is fairly readable, it can be improved.
if (rolls[frameIndex] + rolls[frameIndex + 1] == 10) // spare
{
score += 10 + rolls[frameIndex + 2];
}
else
{
score += rolls[frameIndex] + rolls[frameIndex + 1];
}
Simply extract the logic contained in the boolean expression to another method, allowing the reader to understand the conditional logic from a conceptual point of view - in this case bowling a spare.
if (IsSpare(frameIndex))
{
score += 10 + spareBonus(frameIndex);
}
else
{
score += 10 + sumOfBallsInFrame(frameIndex);
}
Most modern development environments have built-in refactoring utilities that make this kind of operation trivial.
Negative Conditionals
Negative conditional logic can also obscure the meaning of an expression by creating cognitive dissonance that requires the reader to interpret. This wasted time could be alleviated by re-stating the condition in positive terms by creating a method to return the opposite of the intended expression.
For example,
if (!String.IsNullOrEmpty(value))
doSomething();
could very easily have been written as the following.
if (ContainsText(value))
doSomething();
For more information on making your code readable, check out Bob Martin's book, Clean Code: A Handbook of Agile Software Craftsmanship