ruby-style-guide
ruby-style-guide copied to clipboard
Spooky variable declaration
Weird things can happen when you declare a variable within a conditional that you intend to use outside of it. Example:
if false
x = 1
end
puts x # nil, even though the code declaring it was never executed
puts y # crash
See http://stackoverflow.com/questions/12928050/why-does-ruby-seem-to-hoist-variable-declarations-from-inside-a-case-statement-e .
I propose variables should never be declared for the first time within a conditional if you intend to use them outside.
Bad (works, but looks so scary):
if false
x = 1
end
puts x # nil
Bad (works, but if you made a mistake in your logic you get the previous scenario, so it still looks alarming / requires too much inspection to figure out why it won't blow up):
if false
x = 1
else
x = nil
end
puts x # nil
Good (self-evidently safe in a way that doesn't rely on weird parser quirk):
x = nil # Declare the default value for the variable outside the conditional
if false
x = 1
end
puts x # nil
It can be just classified as yet another feature of Ruby that makes code more concise.
Why should one write an additional line of code if it works without it?
Why should one write an additional line of code if it works without it?
Because sometimes it looks really confusing though your words make sense.
Doesn't it make you do a double take every time you see that a variable may not be declared; knowing that at times undeclared variables will crash Ruby?
Scary feeling == bad style. There are plenty of recommendations in this style guide that add a few more characters of verbosity.
This for example does crash:
puts x
x = 1
So it seems to me the parser's behavior in spookily declaring variables that are out of the execution flow, is rather arbitrary/ill-defined, even though it does handle the common gotcha of declaring inside one fork of a conditional.
Or maybe it's just me, I'm also curious about the discussion / what others think.
Agreed (with @danmichaelson and @freemanoid). @abotalov is perfectly correct when he notes that it's a "concise" feature of the language; if we hadn't all seen cases where Ruby isn't so consistent about avoiding crashes, we'd be less spooked by this.
x = if false
1
end
puts x
or
x =
if false
1
end
puts x
I agree with @danmichaelson that this looks a bit weird, especially to people coming from other languages. I'd certainly encourage people to declare variables in a more obvious fashion. :-)
@bbatsov
I'd certainly encourage people to declare variables in a more obvious fashion.
Fervently agreed. Perhaps a gentle reminder in the Style Guide that inspires a warning-level diagnostic in RuboCop and similar? E.g.,
Care is recommended when declaring and assigning to variables. If their only assignment is within a block, consider explicitly assigning
nilto the variable immediately before the block, so the reader understands that s/he isn’t missing a previous assignment.
Possibly leading to a tool diagnostic such as
Style/AssignmentReadability: Variable ‘x’ appears to be declared within a block and used outside it. Assigning nil before the block will produce clearer code and silence this warning.
Thoughts?
No objections from me. It'd be nice to add some lint cop in RuboCop flagging such issues.