The very nice-seeming Gerry has posted an interesting question over on his blog. Basically, it boils down to whether it’s “okay” to add a `#[]` method to the `NilClass` class in the Ruby core, to avoid having to check for the return value of lookups in nested hashes.
His solution boils down to basically this:
class NilClass; def []; nil; end; end
This lets you arbitrarily nest failing lookups in hashes or arrays without a runtime error. It also changes the informal protocol of a core class (`NilClass`, in this case) and makes it “duck-type” much more like a collection.
Generally, I’m in favor of hacking up the core runtime, but in this case I think that monkeypatching `NilClass` to save yourself a few keystrokes is gonna hurt a lot more in the long run than just doing the extra check.
It actually kind of reminds me of a project I worked on once where the “lead engineer” was too lazy to check for null values in his C++ XML parsing code when doing deep tree traversals.
His solution was to trap `SIGSEGV`, then litter invocations of this godawful macro that called `setjmp` before doing any “risky” parsing calls. If the code segfaulted due to null references, he just `longjmp`’ed back to the point where the macro had been invoked, and hoped that any stale references in the heap got magically cleaned up on their own.
Personally, I tend to jump through the extra hoops, and write something like the following:
user = params[:user]
@name = user[:name] unless user.nil?
…which is longer, but makes the intent clear. I’m a much bigger fan of using explicit calls to `#nil?` instead of relying on the overloading of `nil` to mean boolean false.
@David: the problem with checking for return values was having to write a *lot* of code like the following:
DOMNode *child, *child2, *child3; char *data; child = parent.getElementByName('foo'); if (child) { child2 = child.getElementByName('bar'); } if (child2) { child3 = child2.getElementByName('whatever'); } if (child3) { data = child3.getAttributeValue('some-attr'); }Instead, what he wanted to do was this:
DOMNode *child; char *data; child = parent.getElementByName('foo').getElementByName('bar').getElementByName('whatever'); data = child.getAttributeValue('some-attr');…without checking to make sure that none of the intermediate calls to getElementByName returned a null value.
It was sheer laziness, and a mistaken impression that SIGSEGV was equivalent to Java’s NullPointerException. The latter can be caught, and execution can be safely continued; the former means “oh crap, we’re shutting down now” and is a sign that your program is broken.
you could use something like the null object pattern here, but more controlled than by monkeypatching nil.
just define your hash with a default value other than nil, and make it so the default value is always empty.
it turns out you have to make a custom class or else the nil protection wears off after a few levels.
class DeepEnd < Hash
def default(k=nil)
DeepEnd.new
end
end
and gives you this
h = DeepEnd.new
h[:a][:b][:c][:d][:e] #=> {}
No nil hacking required.
Here's my post of it: Off the DeepEnd
Man, and I thought I’ve seen bad code… That setjmp / longjmp example takes the cake!
What would be the problem with checking for the return value?
That is pretty bad.
Hey Lennon,
A similar, worse extension of
NilClasswas actually included in ruby2ruby up until 1.1.7, believe it or not.Check it out:
http://blog.codahale.com/2007/12/17/nilclass-devourer-of-errors
I’m involved with the Sequel and Merb dev teams, and there was a lot of cleanup required when this behavior was removed.
I’m OK with extending base classes, but changing the behavior of the most common guard values is—as you said—gonna hurt like hell in the long run.
—Coda