If key has not been defined, Hashie::Mash should raise NoMethodError instead of returning nil

[carltonbrown]

I've found this class to be very useful, but one thing I don't like is that an undefined key does not cause an exception to be raised. I can't tell the difference between a key that's been deliberately set to nil from a key that wasn't set at all, and this ambiguity makes it difficult to track down where errors were introduced (or not). Please consider pulling! Thanks.

[garrettux]

+1

[carltonbrown]

I fixed 5/6 of the tests that broke as a result of this change, and checked them in... however I'm not sure what to do about the failing spec "should accept a default block". I don't understand the use case of initializing with a block, and I don't understand if it's really important that the default field be initialized to nil. Any thoughts?

[cmar]

I like the default behavior because I never know whats coming back from JSON that I'm parsing. Could you maybe add this behavior as a subclass of Mash?

[carltonbrown]

Doesn't the default behavior make that problem worse though? You can't really know whether the nils came from your source data or Mash spoofed them for you.

[cmar]

The scenario is that I write code to display values that come back from an external web service. If that service occasionally provides an attribute, I can display it on my page without worrying about an exception. I've run into services that don't provide default nil values for undefined, they just don't provide that attribute.

I think your exception is a great idea in some cases. Thats why I was suggesting to put it in a subclass.

Don't forget you can always check if the key exists with .key?

[mbleigh]

Sorry, but I have to disagree here. A Mash is a Hash with method-based access to its keys. Therefore it should behave in the same manner a Hash would if you access a key that isn't initialized. And, in fact, if you strongly desire this functionality you can have it using the default setting block initializer:

hash = Hashie::Mash.new{ raise NoMethodError }
hash.abc # => NoMethodError
hash.abc = 1234
hash.abc # => 1234

As a result, this is not going to be merged. Thanks for the discussion though.

[carltonbrown]

Fair enough. The default block will get it done for me.