Addition of 'deep_assoc` method to Hash in core_ext

rubyonrails-core
1

Hello,

I wanted to see what people thought about adding a “deep_assoc” method to Hash in core_ext.

Given a hash like this:

hash = {a: {b: {c: :d}}}

Its usage would look like this:

hash.deep_assoc(:a, :b, :c)

=> :d

hash.deep_assoc(:a, :e, :f)

=> nil

It is basically a much cleaner way to avoid this pattern:

hash.try(:, :a).try(:, :b).try(:, :c)

It is the one thing which I find myself continually missing in Hash.

A basic implementation looks like this:

class Hash

def deep_assoc(*keys)

current = keys.shift

return self[current] if keys.empty?

return nil unless self[current].is_a?(Hash)

self[current].deep_assoc(keys)

end

end

Please let me know your thoughts.

Thanks!

Micah Buckley-Farlee

2

I would use this often. It seems more like #fetch than #assoc though; would it be better to name it #deep_fetch and have it follow the same conventions re: KeyError or default value?

-Mike

3

I think we had this same request several times and we always rejected it.

I never had to use this pattern and I believe it may be a code smell, but I’m not sure about your usage, so it would be great if you explain why you have this pattern in your application and how often.

The reason that I’m asking that we are in a kind of “code freeze” of Active Support core extension and we avoid the maximum to add new stuff there unless it is really well justified common pattern or it will be used by the framework itself.

I also recommend you to propose to Ruby itself, if we could get it accepted in the language itself we can backport the feature inside Rails.

This is the process that we are even using for methods proposed by Core team members. David proposed Object#itself and we also implemented it inside Ruby 2.2. That way we were able to remove the Rails implementation after we required only Ruby 2.2+.

4

It comes up frequently if you’re consuming XML. It’s not uncommon to receive something like this in the happy case:

Value

And something like this in the unhappy case:

No foos

It’s cleaner to do something like:


errors = Array.wrap(xml.deep_fetch(["FooResponse", "Errors", "Error"], []))

than


errors = Array.wrap(xml.try(:[], "FooResponse").try(:[], "Errors").try(:[], "Error"))

-Mike
5

Thank you for the feedback!

I think the most common use case for me is consuming external APIs, in which case it presents itself fairly often. (It seems particularly endemic to XML APIs.)

There, it is often the case that you need to retrieve a deeply nested value, but cannot rely on its presence.

It was also inspired by clojure’s assoc-in [1], which does the same thing and which I now miss in Ruby.

I certainly understand the extreme reticence to add anything to core extension, but I thought the simplicity and potentially high utility here would perhaps justify it.

Thanks for the suggestion to propose it to Ruby as well, I will do that.

[1] assoc-in - clojure.core | ClojureDocs - Community-Powered Clojure Documentation and Examples

6

-1 on this.

As you mentioned earlier, I’ve seen this pattern so often when parsing XML data, and replaced every single usecase with one of the followings:

In general, hash creation from a XML file using a DOM tree is quite slow and quickly becomes a performance bottleneck. It’s recommended to write a SAX parser when it has be fast. When performance is not a big problem(e.g. testing), consider using xpath. Then the code that reads the xml value above:

xml = response.body

xml = convert_xml_to_path(xml)

errors = Array.wrap(xml.try(:, “FooResponse”).try(:, “Errors”).try(:, “Error”))

will become:

xml = response.body

dom = Nokogiri::XML(xml)

dom.xpath(“FooResponse/Errors/Error”)

Which looks cleaner than the original proposal here. To me, it seems like deep_assoc is trying to solve a problem that is already solved.

I’ve never seen this pattern except when parsing XML and this pattern shouldn’t be used.

7

I’m -1 on adding this as well. I agree with Yuki that for XML traversing it’s better to use something like Nokogiri.

For other use case that you have to go deep down the chain, I think you’d better off creating a class that represent that data structure and maybe use a null object pattern for those methods that could return nil, but still wanted to be chainable.

-Prem