I am on record as believing the
unless...else construct in Ruby is an abomination. I thought I’d dig into that a little more, and explain why even the best-justified uses of
unless...else make for questionable code construction.
Mislav makes as a good a case for
unless...else as any I’ve seen.
unless response.redirect? # process response.body (the more common case) else # follow response['location'] end
The purpose of using
unless here is to put off the less common “special case” of redirection to later in the method. This is a good practice; the common case should always take the spotlight in a carefully constructed method.
This is trivially transformed into an
if...else with negation, of course:
if !response.redirect? # process response.body (the more common case) else # follow response['location'] end
I find this slightly more readable. But that’s not what this post is about, because the
if...else version has the same fundamental problem as the
To explain what I’m talking about, allow me to present a dramatic reenactment of my thought process as I read either of these code examples:
- Line 1: Looks like we’re doing something with responses.
- Line 1 cont’d: Hm, there’s a special case I need to be mindful of involving redirects.
- Line 2: Ah, but this here is the non special-case. Right? Yeah, right, this is the common case. But I’m still keeping that special case in the back of my mind.
- Line 4: Aha, this is that special case that was mentioned at the beginning. What was the case again? Oh yeah, if the response is a redirect.
- Line 4 cont’d: OK, now I understand what to do in the special case.
- Line 5: Wait, what was the common case again?
The problem with this code is that despite trying to push off consideration of the special case to later on, it forces me to think about that special case right at the beginning. Then a part of my mind is waiting in suspense until the case is resolved. And since the condition and the handling are separated by the length of the common-case code, I may have forgotten what exactly the condition for the special case was by the time I get there.
All of this may happen in a matter of seconds, of course.
In Confident Code, I talk about maintaining the narrative flow of a method. I’d argue that the narrative here is unnecessarily convoluted.
It’s not always convenient to push special case processing to the end of a method. When it isn’t, I prefer to dispense with the special case quickly and completely before moving on to the common case. For the example given above, we could accomplish this with a guard clause:
return follow(response['location']) if response.redirect? # process response.body (the more common case)
Here’s how I read this:
- Line 1: Hmm, looks like we’re dealing with responses
- Line 1 cont’d: OK, there’s a special case here involving redirects. Looks like that case is completely dealt with here.
- Line 3: Ah, here we go, this must be the common case…
In this version I’m still confronted by the special case at the start of the method. But the guard clause gives me closure; once I’m done reading it, I feel like I can stop thinking about it completely.
Even if, unlike me, you find
unless...else readable, the problem with it is that even in the best case it is used to separate special-case conditions from special-case handling. I don’t think this lends itself to readable (or refactorable) code. In revisiting the question, I find that my position is still to eschew the use of
unless...else in all cases.
EDIT: To be clear, I’m only talking about
unless...else here. I have no fundamental problem with
unless used without an