Checking for a boolean condition to return a boolean value is awkward
TL;DR: Don't return explicit booleans. Most boolean usages are code smells.
Problems
Declarativeness
Ninja Code
Implementative solutions
Solutions
Return a boolean proposition instead of checking a negation.
Answer must be a business logic formula, not an algorithm.
Context
When dealing with boolean formulas, it is more readable to show a business boolean formula than introduce a negated IF clause.
Programmers tend to return accidental implementative solutions instead of real business rules.
Sample Code
Wrong
function canWeMoveOn() {
if (work.hasPendingTasks())
return false;
else
return true;
}
Right
function canWeMoveOn() {
return !work.hasPendingTasks();
}
Detection
[X] Automatic
Based on syntax trees, we can safely refactor the code.
Tags
- Boolean
Conclusion
Beware of returning booleans.
After the return, you will need an If statement which is also a code smell.
Relations
More Info
Credits
Photo by Morgan Housel on Unsplash
Thanks to Nico K. for this suggestion.
It's not at all important to get it right the first time. It's vitally important to get it right the last time.
Andrew Hunt
This article is part of the CodeSmell Series.
Top comments (5)
I need to stop reading this series…my code doesn’t smell it stinks I realise as I seem to break every rule 🤣❤️
Articles are guides and suggestions.
Don't take it too hard
I was kidding, I am enjoying the series, it was self deprecating humour! (Or meant to be 🤣)
I have and I am learning a lot from these series. @mcsee a question please (your opinion)
say we have the code:
All we want is the result of
work.hasPendingTasks()and in this post, you suggested that it is better not to explicitly return a boolean.So you refactored it to:
Now, the refactored code looks like a wrapper around
work.hasPendingTasks();and question is that, when it happens that we have a simple single lines like, it is okay to wrap them up? I mean, we could used thework.hasPendingTasks();directly. What do you think or encourage? ThankscanWeMoveOn the 'what'
hasPendingTaks the 'how'
they are accidentally coupled, now. But the caller to 'canWeMoveOn' does not know,
If you change your implementation or add new business rules or add a cache the caller should not be aware
I used this example here
Code Smell 123 - Mixed 'What' and 'How'
Maxi Contieri ・ Mar 22 ・ 2 min read