Is assignment in a conditional clause good ruby style?

It is GOOD style to use assignments in conditionals. If you do so, wrap the condition in parentheses.

# bad (+ a warning)
if v = array.grep(/foo/)
  do_something(v)
  # some code
end

# good (MRI would still complain, but RuboCop won't)
if (v = array.grep(/foo/))
  do_something(v)
  # some code
end

# good
v = array.grep(/foo/)
if v
  do_something(v)
  # some code
end

See the community style guide for more information


One somewhat widespread idiom is to use and, which would look something like this:

tmp = method_call_that_might_return_nil and do_something_with tmp

Another possibility would be to call #nil? explicitly, that way the intent becomes a little bit clearer; in particular it is really obvious that you actually meant to assign instead of compare:

unless (tmp = method_call_that_might_return_nil).nil?
  do_something_with tmp
end

Concise code is not necessarily better code. Concision is useful when it improves the communication of intended code behavior from author to future maintainers. I think enough of us come from backgrounds in which we've had accidental assignments in if blocks (when we meant to have an equality comparison) that we prefer styles in which it's absolutely clear that assignment is meant, rather than comparison. The .nil? idiom already mentioned has that property, and I'd consider it cleaner than having the bare assignment inside the if condition. Really, though, I don't see the harm in having the extra line of code for the assignment.


The functional-programming way to do this is to use andand. It's a readable way of chaining method calls so that a nil in the middle stops the chain. So your example would be something like:

method_call_that_might_return_nil.andand.tap {|obj| do_something_with obj}
## or, in the common case: ##
method_call_that_might_return_nil.andand.do_something

Yeah, I would say it's bad style due to the possible confusion between assignment and comparison. It's only one more line to assign and then test, and it avoids having someone in the future think that was a bug and patch it to use == instead.