You've all seen the literal side mountain of code, here's a refresh:
if (candies.length > 0) {
if (store.isOpen()) {
if (store.allCandyIsLow()) {
for (let candy of candies) {
store.refill(candy);
}
notification('Store ' + store.Name + ' had its candies refilled.');
} else {
console.error('There is still candy in the store');
}
} else {
console.error('No stores to put the candy into');
}
} else {
console.error('No candies');
}
This becomes really difficult to manage because
- code block's main purpose is nested inside all if statements while
- the exceptional/error cases are right in your face and
- their condition and error are as far away from each other as possible
This hurts the code's readability, flexibility and maintainability (adding or changing the condition's order becomes tough).
Here's a practical solution that can be applied at any moment in development you want with doing these simple steps until there are no longer any exception cases nesting the main code:
- take the code out of the if statement
- invert the condition
- bring the code inside else in the if block
- add a return statement (or whatever comes after the large block)
The code from above should now look something like this:
if (candies.length <= 0) { // or === 0 in this case
console.error('No candies');
return;
}
if (!store.isOpen()) {
console.error('No stores to put the candy into');
return;
}
if (!store.allCandyIsLow()) {
console.error('There is still candy in the store');
return;
}
for (let candy of candies) {
store.refill(candy);
}
notification('Store ' + store.Name + ' had its candies refilled.');
Albeit, the code block itself became larger but
- there are no longer any nested if statements
- the main code comes right after all the exception cases
- you can see what each exception case does and add/modify as needed
What do you think? Are there any better solutions? Does this one have some critical flaws we missed? Ask away!
Top comments (8)
I used to call such a code constructions
pyramids.They are so stable like on the picture above.
The true term is actually called guard clause : medium.com/@scadge/if-statements-d...
And as we don't nest conditions, it is not a pyramid, it is flat ;)
That's true. I’m familiar with the term ”guard clause”. It’s a part of defending coding style :) To be clear, I have nothing against guard clauses, I use them very often, sometimes I extract them to separate class to split validation logic into a separate piece of code. To keep the code clean, we can't nest if statements. As you said ”flat” construction is not bad, but the pyramid is, in my opinion, the pure evil.
Agree at 100%
This is my coding style too.
It isn't more code. Same amount of lines actually. And way more readable. :)
Hah! Good catch. Did not even notice. Maybe because of the added return statements.
Great article! Sometimes making compact code isn't the best strategy.
Thank you!
My opinion is that less code = less headaches later on with some exceptions like this one. Of course, don't need to make it as compact as possible until everything is crammed inside one line. Where you can, cut quite a few lines of code.