Ruby early return considered harmful (for idiots like me)
I've been playing with some Rails code at home to support some of the work we do at the day job. I'm building a search tool that detects the kind of input and performs searches based on this type. I've been working with Ruby's regular expression support, and test driving a class to find UUIDs. Here's what I started with (please don't comment that it could be simpler, I was starting simple... :).
class Uuid REGEX = /[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}/ def Uuid.isUuid(fragment) if fragment =~ REGEX return true end return false end end
Thinking that this could be simplified be removing the return
keyword, I removed it!
class Uuid REGEX = /[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}/ def Uuid.isUuid(fragment) if fragment =~ REGEX true end false end end
This caused all sorts of funny problems, which in hindsight are painfully obvious, but at the time are just a pain. Removing the return causes the if
to essentially fall-through, always
returning false
.
Discussing this at work, perhaps I've hit up against a potential maintenance issue with Ruby. I'm hoping this isn't the case (you can do similar stupid things in most languages), but some of the things you can do with Ruby make me a little nervous. I'm holding my tongue for a while (I did the same thing with XP and am hooked) to see where things go, I'm sure it'll all be fine...
9 Comments:
OK, I know nothing of Ruby, but I'm wondering why you chose to use the if statement at all. What's wrong with:
def Uuid.isUuid(fragment)
return fragment =~ REGEX
end
For that matter, since it seems to use the last evaluated value as the return value, how about dropping the return?
def Uuid.isUuid(fragment)
fragment =~ REGEX
end
I was playing around with it basically. I went from the simplest (what you suggested), to the long hand form after hitting trouble. I'm still not convinced that the regex operator is behaving correctly (where correctly means my interpretation of it :), so I haven't dropped the true/false bits yet. The operator does more that just return true or false, it returns nil (i.e. null) if it doesn't find a match, or the index of the match if it does find it. I'm not sure how this behaves when used in an if.
Luckily I have test-driven the code, so I just changed it and it still works! Cheers.
You probably know this by now:
The =~ operator returns the character position in the string of the start of the match (which evaluates to true in a boolean test), or nil if no match was found (which evaluates to false).
See here
Regular expression is really wonderful to parsing HTML or matching pattern. I use this a lot when i code. Actually when I learn any new langauge, first of all I first try whether it supports regex or not. I feel ezee when I found that.
http://icfun.blogspot.com/2008/04/ruby-regular-expression-handling.html
Here is about ruby regex. This was posted by me when I first learn ruby regex. So it will be helpfull for New coders.
Regular expression is really wonderful to parsing HTML or matching pattern. I use this a lot when i code. Actually when I learn any new langauge, first of all I first try whether it supports regex or not. I feel ezee when I found that.
http://icfun.blogspot.com/2008/04/ruby-regular-expression-handling.html
Here is about ruby regex. This was posted by me when I first learn ruby regex. So it will be helpfull for New coders.
Thank you for supplying the regex for parsing/validating a UUID. Google brought your page up first for "regex uuid".
But claiming that early returns are bad because when you remove the return keyword it doesn't work? Come on now. Sheesh.
Scott, it's me poking fun at myself, it's not serious.
Interesting article, added his blog to Favorites
old post. ok whatever.
as a rule of thumb, never use more than 1 return statement in a function. first thing I was thought.
S.
Post a Comment
<< Home