Another silly coding error

30 Mar 2017

I was working on a Ruby thing the other day and found a bug in my code. I was trying to match when blocks on multiple values in a case statement, so I had written:

case foo
  when "bar" || "buz"
      do_something
  when "biz" || "baz"
      do_something_else
end

See the bug? I had intended to call do_something if foo was either "bar" or "buz". But what I was actually doing was only checking if foo was "bar", because "bar" || "buz" is an expression that evaluates to the first non-nil literal, "bar". So the "buz" alternative was never being checked. I should have been using a variable arguments splat-ish thing:

case foo
  when "bar", "buz"
      do_something
  when "biz", "baz"
      do_something_else
end

This bug was pretty devious; it flew under the radar for quite a while. I introduced it at least 5 years ago; it was there when I switched this project's repo from Subversion to Git in 2012, and since I didn't import history who knows how long it was present before that. A big takeaway is that I'm missing some tests. An interesting thing there is that to see that, I'd have to do not just line-oriented coverage checks but branch coverage. Not a great excuse but hey I'll take it.

What's funny about this bug is that the original code - although obviously wrong - feels right-ish to me. I've been trying to figure out why it looked right. Maybe that's mostly because it's syntactically valid. But I think it's also because it looks vaguely like the way things would work if the when reserved word accepted a block and evaluated that block against the case expression; something like when { "foo" || "bar" }. Interestingly, when does accept a block and yields the variable to it, so this would have worked:

case foo
  when ->(x) { x == "bar" || x == "buz" }
      do_something
  when ->(x) { x == "biz" || x == "baz" }
      do_something_else
end

That's fairly close to my original incorrect code. Not quite the same, and much clumsier than a varargs usage, but, there we have it.

So the takeaway is gosh, programming, am I right?