aetherknight / recursive-open-struct

OpenStruct subclass that returns nested hash attributes as RecursiveOpenStructs
Other
276 stars 54 forks source link

Hash#dig and Hash#fetch patches #56

Closed maxp-edcast closed 5 years ago

maxp-edcast commented 6 years ago

Took me a bit of time to realize my code was failing because RecursiveOpenStruct#dig (inherited from Hash) returns a plain Hash.

struct = RecursiveOpenStruct.new(a: {b: 1})

struct.dig(:a, :b)
# => {:b => 1 }

struct.dig("a", "b")
# => nil

I am using the following patch:

class RecursiveOpenStruct
  def dig(*keys)
    keys.reduce(self) do |memo, key|
      val = memo[key]
      val.is_a?(Hash) ? self.class.new(val) : val
    end
  end
end

x = RecursiveOpenStruct.new a: {b: 2}

x.dig :a, :b
# => 2

x.dig "a", "b"
# => 2

What are your thoughts on adding this function here?

While I'm at it, I also added a RecursiveOpenStruct#fetch method, because I found myself using it in code before realizing it didn't exist:

class RecursiveOpenStruct
  def fetch(*args, &blk)
    to_h.fetch *args, &blk
  end
end
aetherknight commented 6 years ago

@maxp-edcast I'll take a look at adding special version of#dig to help it work better on Ruby 2.3+. However, #fetch is not yet a part of OpenStruct's API (as of Ruby 2.5: http://ruby-doc.org/stdlib-2.5.0/libdoc/ostruct/rdoc/OpenStruct.html), and I am reluctant to add new public methods that prevent attributes/properties from being used unless OpenStruct (or Object or BasicObject) has also done so.

A bit of rumination:

aetherknight commented 6 years ago

I have added improved #dig support so that ROS properly recurses over both Hashes and Arrays (when recurse_over_arrays: true) in v1.1.0.

maxp-edcast commented 6 years ago

awesome thanks.

maxp-edcast commented 6 years ago

Not sure why, but when I use the dig code you added I get an error from the name_val = self[name] line, it says undefined method has_key? for #<RecursiveOpenStruct>

To fix this I added the following patch:

def has_key?(key)
  to_h.has_key?(key)
end
aetherknight commented 6 years ago

Can you provide a code sample, Ruby version, and full error stack trace? I don't know under what circumstance that has_key? is being called on an ROS (this sounds like it might be bug such as trying to initialize a RecursiveOpenStruct with another RecursiveOpenStruct instead of a Hash). OpenStruct does not implement has_key? so it should not be a part of RecursiveOpenStruct's APIs.

maxp-edcast commented 6 years ago

Yes you are correct, it was because I was passing a RecursiveOpenStruct to RecursiveOpenstruct.new. Is there a particular reason this is not supported? Is it a result of the desire to have as little an API footprint as possible?

aetherknight commented 6 years ago

In general, ROS tries to be a strict extension to OpenStruct, and OpenStruct only accepts a hash as its input.

While I could do something like have the initializer for ROS call #to_h on whatever object it is given (changing the initializer's signature from accepting an optional Hash to accepting any object that implements to_h that returns a Hash), this might not always have the result that the caller desires since it is often a shallow conversion to a Hash (eg, compare that to ActiveModel objects that use #as_json to get a "deep" Hash representation of objects appropriate for JSON serialization). So I'd rather keep the initializer interface simple and mirror OpenStruct's behavior instead of changing the API to require the caller to provide something that implements the to_h implicit protocol. The caller should know what it wants and can easily call to_h (or to_hash or as_json or anything else that converts at least the immediate object into a Hash representation)