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 unless...else
version.
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 else
clause.
I do a slightly different thing:
follow(response[‘location’]) and return if response.redirect?
# process response.body (common case)
I find it a bit more readable than beginning the line with return.
I can understand that point of view. My perspective is: an early return is potentially one of the most surprising things to find in a method, so I want it to be very, very obvious that a line may cause an early return. Hence, starting with the return.
Ah, but an early return is only surprising if you don’t use early returns a lot – and it’s one of the few cases where I think the old-style one-statement-per-line adds clarity. I’ve always been a fan of “straight line” programming, from the days before exception handling:
if no_disk_space
deal_with_it
return
end
if unexpected_data
deal_with_that
return
end
do_the_real_thing
for_30_lines_or_so
I’m probably biased from reading my own code, but 20 years after I wrote it, I have no trouble following the narrative – the narrative is “skip over the guard clauses and find all the stuff in column 1”. e.g.: https://gist.github.com/2259544
BTW, there appears to be a bug in your code. The “and return …” will only be executed if the follow() returns a truthy value; I doubt that’s what you want.
You are totally right. It was working because all the methods I used this with returned truthy stuff. So it was a hidden bug. Your idiom is better. Thanks!
Ah, good to read about this. Never thought about that ” … and return” only returns if the first one does not return anything falsy. May have to check some code.
I’m a Ruby beginner and I was thinking the same thing. Then I also end up using this way you’re describing too. I guess it comes naturally. Thanks.
I write special cases in the same way, but I think there’s still room for the if…else…end construct, because sometimes it means either/or instead of a common case and special case. You could argue that that’s what the ternary operator is for, and you’d be correct. But for some constructs, I find the ternary operator to be less readable. For example, when the ternary’s guard is a method ending with a question mark, I find the if…else…end construct more readable than having the two consecutive question marks.
This post is in no way against if…else. Only if!…else, aka unless…else.
Oh, my mistake. Thanks for clarifying that point.