Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hash#dig and Hash#fetch patches #56

Closed
maxp-edcast opened this issue Feb 2, 2018 · 7 comments
Closed

Hash#dig and Hash#fetch patches #56

maxp-edcast opened this issue Feb 2, 2018 · 7 comments

Comments

@maxp-edcast
Copy link

maxp-edcast commented Feb 2, 2018

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
@maxp-edcast maxp-edcast changed the title Hash#dig patch Hash#dig and Hash#fetch patches Feb 2, 2018
@aetherknight
Copy link
Owner

@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:

  • RecursiveOpenStruct#dig is not inherited from Hash but from OpenStruct. OpenStruct's implementation conforms to the same protocol as implemented by Hash, Array, and Struct, allowing a call to #dig on one to inter-operate with the others.

  • I'm not super thrilled about yet another method name being reserved and thus unusable when it is a key in the Hash used to initialize the RecursiveOpenStruct. However, it is part of OpenStruct's API in Ruby 2.3 and newer.

@aetherknight
Copy link
Owner

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
Copy link
Author

awesome thanks.

@maxp-edcast
Copy link
Author

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
Copy link
Owner

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.

@aetherknight aetherknight reopened this Feb 5, 2018
@maxp-edcast
Copy link
Author

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
Copy link
Owner

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants