Monday, January 30, 2012

The dangers of ||= for booleans in Ruby

The ||= operator in Ruby is a fantastic shortcut to initialize a variable only once. It is used all the time to write something like:
    some_hash = {} if some_hash.nil?
into a much shorter and more compact:
    some_hash ||= {}
However, it is extremely important to NEVER use this operator to initialize a boolean flag this way. In fact, it is tempting to write something like:

    def exists?
        @exists ||= record_exists?
    end
To cache the boolean return value of record_exits?

This code seems to say "if @exits is null, then calculate it and return it; otherwise, return the cached value that we already have".

However there is a fatal mistake here. The ||= operator evaluates the rvalue if the lvalue is nil OR FALSE! That means that this code will end up calling record_exists? every time that @exists is un-initialized (good), but also any time that @exists is false!

Not what you wanted!

So... in general, NEVER use ||= to initialize un-initialized booleans. Stick with:
    def exists?
        @exists = record_exists? if @exists.nil?
    end

2 comments:

  1. Your advice is a little off. It should be "never use ||= to initialize a boolean flag IFF the default is true".

    Obviously, `a ||= false` is just fine.

    ReplyDelete
  2. Ryan,

    The usage pattern I was thinking of (see examples) is with an rvalue that is not a constant, but the result of some calculation for example. In other words I was more thinking it as a cache for a value, and less as an assignment of defaults.

    Yes indeed if the rvalue is and is going to stay "false", you can use ||=.

    However I would still not recommend doing it. Here is why:

    Code gets re-factored and what today is a "false" might end up in the future being a method call or a variable. An easy path is when that "false" becomes a constant and then becomes a parameter or option for example. (just to name a very common code evolution pattern.)

    It would be a very easy thing to miss when you read the code because of how usually people (or at least I) are used to read ||=, that is thinking it as a substitute for "if it is uninitialized (nil), then initialize".

    Of course if your test coverage covers it you are going to catch that, but tests rarely cover 100% of the execution paths and subtle bugs can be introduced with that use pattern.

    So, regardless if in some cases it actually does work, I still think that using ||= for booleans is a bad idea and source of easy mistakes and possible bugs. I'd just avoid it all together.

    ReplyDelete