Recursively converting child hash attributes to OpenStruct.

[abhionlyone]

This PR can enable something like this:

person = OpenStruct.new
person.name = "John Smith"
person.age  = 70
person.more_info = {interests: ['singing', 'dancing'], tech_skills: ['Ruby', 'C++']}

puts person.more_info.interests
puts person.more_info.tech_skills

[abhionlyone]

Hi @hsbt , Did you get a chance to review this PR? What do you think about the changes?

Regards,
Abhilash

[esparta]

No maintainer or someone to have decision, but before going forward I'd suggest two things:

• A set of benchmarking on this. How this will impact the current code.
• Tests

Additionally, as being a new feature this should be proposed via https://bugs.ruby-lang.org ticket, will be discussed there (if not rejected before).

[marcandre]

It's fine to propose a feature/change without having tests, etc., as a way to first check if the feature is acceptable or not.

In this case I believe it isn't, for compatibility reasons. OpenStruct doesn't respond to all the methods of Hash, so if someone is already storing a hash in an attribute of an OpenStruct, she is expecting that attribute to be a Hash.

In the example above, person.more_info.transform_values{ ... } would no longer work.

[abhionlyone]

@marcandre Thanks for the response!! I completely agree with your feedback. But there are a lot of people out there expecting OpenStruct to recursively convert a child hash to another OpenStruct object. So, why not provide them with an option to recursively convert child hash objects to OpenStruct object? Maybe allowing users to pass additional options to constructor would help?

Like this:

def initialize(hash=nil, options={})
  ...
end

[marcandre]

Adding a recursive: true or similar option to the constructor seems to me like a much better idea 😄

The official forum to get a new feature accepted is https://bugs.ruby-lang.org/ . Best thing at this point would be to make the feature request there to see if there are any objections.

If you'd like to work on the implementation, you can amend this PR too. You'll need tests, also I'd use respond_to? :to_hash as a check.

[abhionlyone]

@marcandre Thanks! Like you suggested I will create a feature request on https://bugs.ruby-lang.org/ 😃

[nobu]

recursive is not defined as a keyword argument.

[marcandre]

As I pointed out, best use respond_to?(:to_hash) instead of is_a?(Hash)

[abhionlyone]

Sorry missed that. Updated the code!

[imnithin]

+1 for the PR

[marcandre]

Would be more readable to use a keyword argument I'd say

[abhionlyone]

Updated! Thanks :)

[abhionlyone]

if we use keyword argument the below test will fail:

def test_accessor_defines_method
    os = OpenStruct.new(foo: 42)
    assert os.respond_to? :foo
    assert_equal([], os.singleton_methods)
    assert_equal(42, os.foo)
    assert_equal([:foo, :foo=], os.singleton_methods.sort)
end

But for better readability I changed the constructor to this:

def initialize(hash=nil, options={recursive: false})
    @table = {}
    @recursive = options.fetch(:recursive, false)
    if hash
      hash.each_pair do |k, v|
        k = k.to_sym
        @table[k] = (@recursive && v.respond_to?(:to_hash)) ? OpenStruct.new(v,true) : v
      end
    end
end

[abhionlyone]

Just FYI, A feature request has been created here https://bugs.ruby-lang.org/issues/15225

[chopraanmol1]

OpenStruct.new(v, options) or OpenStruct.new(v, recursive: true)

[abhionlyone]

Yes, it should be OpenStruct.new(v, options). Will update the PR in a bit.

[chopraanmol1]

Same here
OpenStruct.new(v, recursive: true)

[n-rodriguez]

Hi there! Any news?

[marcandre]

I updated https://bugs.ruby-lang.org/issues/15225