Daily WTF: NilClass#method_missing

Is this your first visit? You may want to subscribe to the feed.

Daily WTF: NilClass#method_missing

With great power comes great responsibility.

I’ve been leading Ruby and Rails training over the last couple weeks for a company in Brisbane, Australia, Xerox in El Segundo, CA, and yellowpages.com in Glendale, CA. Visiting companies is always so interesting because you get to see how different people work together and how they go about solving problems. You also get to see some really interesting code.

I was helping one of the companies walk through some code they inherited from an acquisition and came across something similar to this wonderful snippet of code:

class NilClass
    def method_missing(method, *args)
      raise(NoMethodError, "undefined method '#{method}' for nil")
    rescue => e
      CustomLogger.error(e)
      nil
    end
  end

WTF? They’re overriding #method_missing on nil to raise an exception, they then proceed to rescue it, log the error, and return nil!

I’m guessing they had some buggy code that was calling a method on nil, causing a NoMethodError to be raised. And instead of fixing the code, they just decided to change it so that you could call methods on nil and it would just silently fail.

>> nil.foo.bar.baz
=> nil

It took me a while to figure out why they were raising an error and then rescuing it, but I’m guessing it’s so they have access to a stack trace in the logger.

I’ll give them credit for being creative, but whoever wrote that piece of code doesn’t deserve to be using Ruby. Yes, Ruby’s open classes are powerful, but that’s just ridiculous.

Code: code, nil, ruby, wtf Apr 18, 2008 ● updated Oct 14, 2008 ● 8 comments

8 comments

Apart from the fact the strack trace is always available via caller.

Stephan April 18, 2008 at 03:38 AM
The strack trace is always available via caller anyway.

Stephan April 18, 2008 at 03:38 AM
I completely agree. I found this in LovdByLess (which has been getting a lot of attention lately):

http://github.com/stevenbristol/lovd-by-less/tree/master/vendor/plugins/less_monkey_patching/lib/nil.rb

WTF?

Brian April 18, 2008 at 05:18 PM
Spend enough time with a complex Rails app, and you’ll understand why this happens.

This is often a fairly reasonable thing to do. Log the event, return nil to (usually) the view rendering code so that the app doesn’t 500 for your users, and follow up on the log messages and fix yr damn code. :)

andrew June 24, 2008 at 02:38 PM
andrew:

I’ve spent every day of the last 2 years working on Rails apps. I don’t know that I’ve worked on a project yet that I wouldn’t classify as “complex”.

Why stop at overriding #method_missing on NilClass? Why not just override it on Object so that any object responds to any method? That would make sure that you never get an error message!

If your code is hitting a condition where it would raise an error, then it’s probably not doing what the user expects anyway. Why pretend like nothing is wrong? Your users would appreciate if you quit wasting there time and just acknowledged that there is an error and you’re working on fixing it.

Overriding #method_missing on NilClass in this manner is simply ridiculous. It is the symptom of poor programming.

Brandon June 24, 2008 at 05:10 PM
Nah, you’re missing the use case: sometimes Rails views will 500 due to “impossible” data conditions, far away from the bug (and maybe long after the bug has been fixed). Unless you have an ultra-rich and constantly-running data integrity checker, these unexpected conditions can result in error pages for users when you try to display the username of the owner of a comment on a video belonging to a group owned by a user that might have been deleted.

Careful and judicious use of NilClass#method_missing (with good logging and followup) can save a ton of work. An alternative would be to nil protect everything in views (and then what would you do anyway? Typically return nil or skip the record entirely, which encourages the bug or data corruption to go unnoticed for far longer than reasonable use of method_missing would).

With great power comes great responsibility. Sure. I’d argue that this is a perfect example of a time when, if you’re being greatly responsible, you can use the great power for great benefit.

Andrew August 23, 2008 at 11:30 PM
Oh.. and the raise/rescue is probably just a cheap way to hook an existing ExceptionNotifier method, yet still allow the page to render without error.

Andrew August 23, 2008 at 11:36 PM
I think the implementation is meant to mimic how Objective-C works with nil/null. The cool thing about the core of the idea (returning nil on nil.whatever) is pretty neat in that you dont have to do nil checking. If you have a decision that includes nil.whatever it will always return false instead of having to do my_nil_object && my_nil_object.method_i_intended_to_call.

Its neat and works in objective c, so…

Josh May 26, 2009 at 04:15 PM

opensoul.org