views:

158

answers:

4

I've just started working with Ruby, and discovered statement modifiers when RubyMine suggested I change this code:

if !VALID_DIRECTIONS.include?(direction)
   raise ArgumentError, "Invalid direction"
end

to this:

raise ArgumentError, "Invalid direction" if !VALID_DIRECTIONS.include?(direction)

I like how it makes the code more succinct. However, I can see it potentially misleading at a first glance and imposing a readability issue, because it places the effect before the condition. Then again, maybe that's just because I'm so used to C-style languages.

Has anyone run into trouble as a result of using statement modifiers, or do you feel they have improved your code? Also, does anyone have general guidelines for using the modifiers (i.e., works particularly well for some operations, or not for some others)?

A: 

It's a purely subjective matter of style. Use whatever you feel comfortable with.

Azeem.Butt
+2  A: 

It was a little strange for me at first, but I do not think it poses a readability problem. When working in Ruby a lot, it makes perfect sense. It is only when I switch back and forth with other languages that it is noticeable. However, as you immerse yourself in Ruby code you will find it a clean and succinct way of writing one line conditionals. Also, get used to using unless. Your line of code could (maybe should) be written:

raise ArgumentError, "Invalid direction" unless VALID_DIRECTIONS.include?(direction)
Doug Neiner
+3  A: 

Statement-modifiers make ruby behave more like English, which is nice:

  • if it rains, stay home
  • stay home if it rains

I'd advise you to use the form that seems the most natural and elegant to you. If in doubt, read the statement out loud in both forms. Personally, I tend to only use statement-modifiers for short statements such as return :nope if input.nil? -- for longer or more complicated statements, it can take the reader longer to grasp, because the eyes only cover a certain amount of space and so someone would only read the modifier on second glance.

DataWraith
+3  A: 

I find that I usually have no trouble reading those trailing conditionals (as they are also sometimes called), provided that other code readability guidelines are still followed. Putting a 60 character expression and a 40 character condition on the same line, you are going to end up with a 100 character gob of text, which is surely going to be unreadable, completely independent of the issue of trailing conditionals.

In the specific code sample you are showing, it is pretty much obvious that there must be a conditional following. Who would want to raise an ArgumentError without even taking a look at the arguments first?

Also, trailing conditionals are similar to guard clauses in math and functional languages, which also tend to be written after the expression they are guarding.

Last but not least, putting a couple of raise Bar if foo and return nil if quux expressions at the beginning of methods, as kind of guards, is actually considered good style, to simplify the control flow of the method. Again: since these come at the beginning of the method, it is kind of obvious that there has to be a condition, otherwise returning from the beginning of the method wouldn't make sense.


PS: I would actually use unless there, to get rid of the negation. With more complicated conditions, I find that unless can sometimes be hard to parse, but in this case, it's more obvious, at least IMHO.

Jörg W Mittag