How to Make If Statements More Understandable
If statements are necessary to build our code.
Granted, some ifs are bad, like those that try to emulate polymorphism by testing a series of types. Those, you want to stay away from.
But the ones that implement domain rules are good, and even an opportunity to make your code more expressive by showing how it translates the domain logic. I want to focus on those good ifs, and show how to make them an asset for your code.
In particular, I want to point out that, to have the clearest if statements possible, you shouldn’t make them more compact than they are in the specification you received from the domain.
Packing off the condition
Before getting to this, the classical advice about if statement is to code them at the right level of abstraction. Said differently, an complex association of AND and OR operators can be hidden behind a name that qualifies what the condition is about rather that how it is implemented.
To illustrate, say that for checking if a financial index is valid for saving then it needs to have an ID, be quoted on a market and be liquid on that market. Then the following condition:
if (index.hasID() && index.isQuoted() && index.isLiquid())
{
...
can be rewritten in a clearer way:
if (isValid(index))
{
...
with the isValid function defined as:
bool isValid(Index const& index)
{
return index.hasID() && index.isQuoted() && index.isLiquid();
}
This is not rocket science, but it is useful very often.
The second classical piece of advice about if statement is…
Choosing good names
And in particular avoiding negations in the names of the variables. Pondering over the naming of something in your code? Then you’ll want to check out How to choose good names in your code.
Don’t compress an if statement more than in the spec
Now that the classical advice is behind us, I want to delve into this guideline that I haven’t seen formalized anywhere but that I find very useful, to make if statements more expressive.
What I mean by the specification (or spec) is the set of instructions given by the business to the developers about what they should implement in the application.
Let’s take an example:
A user can subscribe to an event before a certain deadline, which is indicated by a date. For some events the user can apply on the date of the deadline, for some others the day of the deadline is too late. This is my spec. It’s simple, right? (This comes from code I’ve seen in production – if this sounds like an odd story it’s because I’ve stripped out the real domain of the original example and replaced it with this one.)
Now here is one implementation for this:
bool subscribedInTime(Date subscriptionDate, Date deadline, bool strictlyBeforeDeadline)
{
return (subscriptionDate < deadline) || (!strictlyBeforeDeadline && (subscriptionDate <= deadline)))
}
Ugh. Is this correct? My head hurts when I squint and try to run the various cases mentally. Do you think it’s correct?
Well, the code was later changed to that implementation:
bool subscribedInTime(Date subscriptionDate, Date deadline, bool strictlyBeforeDeadline)
{
return (strictlyBeforeDeadline && subscriptionDate < deadline) || (subscriptionDate <= deadline)
}
Is this one better? Or is it equivalent? Frankly, I’m not sure. Only thorough unit testing would tell us this.
Paradoxically, even if there are very few lines of code it takes a lot of time to understand them. And nothing says that a compact if will run faster than a more developed one.
The issue with these implementations is that they try to optimize the code by making the if statement as compact as possible. As a result, it no longer expresses what is in the spec. This is a problem, which leads to the following guideline:
if statements should be as close as possible to their specification.
Can you guess how to use this guideline to make our example code more expressive? Think about it until you come up with a solution. Here is one:
bool subscribedInTime(Date subscriptionDate, Date deadline, bool strictlyBeforeDeadline)
{
if (strictlyBeforeDeadline)
{
return subscriptionDate < deadline;
}
else
{
return subscriptionDate <= deadline;
}
}
The spec was easy to understand. The code should not be harder.
Try yourself on another example
Want to get more practice at this technique? I’ve got another case for you. Once again I’ve stripped off the original domain, but this comes from real code we refactored with my team (thanks Aadam!).
A customer is making a purchase, and we need to write a piece of code that computes the discount to apply on it. Here is the spec:
Some items have a red tag. For those, the price on the tag is the price applied.
Some items are only available on the online store: those can benefit from a special day discount if there is one. Such a discount is an amount off the price (say $3 off) but the price can’t go below a minimum (of say $1).
And the rest of the items can be on sale, with a percentage off the price (say 50% off).
The item has a price_ member, let’s write the applyDiscount method that updates this price (which is perhaps an arguable design but let’s focus on the if statement here).
Here is an attempt that doesn’t respect the guideline of writing the if statement as close to the spec as possible:
void Item::applyDiscount()
{
if (!hasRedTag() && isSoldOnlineOnly())
{
if (hasSpecialDayDiscount())
{
price_ = std::max(minimumPrice, price_ - getSpecialDayDiscount());
}
}
else if (!hasRedTag())
{
price_ *= 1 - getSaleDiscount();
}
}
This code implements the spec correctly, but doesn’t look like it. Indeed, you can see that hasRedTag appears in the else branching dedicated to the sale discount, which is not how the spec is structured. This can throw off someone who’s reading the code.
Can you think of how to modify this implementation to make it more expressive?
Here is one solution:
void Item::applyDiscount()
{
if (!hasRedTag())
{
if (isSoldOnlineOnly())
{
if (hasSpecialDayDiscount())
{
price_ = std::max(minimumPrice, price_ - getSpecialDayDiscount());
}
}
else
{
price_ *= 1 - getSaleDiscount();
}
}
}
What do you think? I find it much clearer as it reflects better the business algorithm explained in the spec.
What’s interesting if that the second (clearer) implementation is more deeply nested than the first one. However nested ifs are known to be a bad thing, right?
Well, not always. If flattening comes at the expense of making the if statement more technical and farther away from the specificaiton, then we’re better off leaving the nested version.
Now sometimes it feels awkward to follow the spec line by line in the if statement. If you’re feeling that way then you should worry about the spec itself. If you think it should be expressed differently then it’s time for a meeting with your business people to validate this, and possibly improve the spec. And the if statement.
Programming is so great! Even writing an if statement is something we can get better at. Write them as close as possible to their specification to curb their complexity and make your code as understandable as possible.
Related articles:
Don't want to miss out ? Follow:
Share this post!
