aetherknight / recursive-open-struct

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

Just curious, but why is the implementation here so complicated? #53

Closed banister closed 7 years ago

banister commented 7 years ago

I'm sure i'm missing something, but why can't you just use a simple recursive function to convert a nested hash to nested open structs? e.g

def convert_to_object(arg)
  return arg unless arg.is_a?(Hash)

  OpenStruct.new.tap do |os|
    arg.each do |k, v|
      case v
      when Hash
        os[k] = convert_to_object(v)
      when Array
        os[k] = v.map { |h| convert_to_object(h) }
      else
        os[k] = v
      end
    end
  end
end

Use it like so:

obj = convert_to_object(a: 1, b: 2, c: { j: 1, k: { u: 3 } })
obj.a #=> 1
obj.c.k.u #=> 3
aetherknight commented 7 years ago

I originally wrote this gem 8 years ago, and it was a relatively simple subclass of OpenStruct back then (although had more lines of code than your example above, since I added the ability to optionally recurse over arrays). Since then, other developers started using it to mutate nested hash values and ran into various bugs with doing that (one of which is still open). Addressing those bugs has added significantly to the complexity of ROS.

I agree that it seems more complicated than it should be, and an approach like your function would work for what I think is probably the majority of use-cases and then. I don't really have the time or inclination to figure out how to best rewrite ROS (if it can be simplified and still mostly conform to OpenStruct's API), and I am mainly just making sure it works with new versions of Ruby and accepting bug fixes.

banister commented 7 years ago

Ah that makes sense, thanks :) However (unless i'm misunderstanding again) the above code also recurses over arrays:

obj = convert_to_object(a: 1, b: 2, c: [{j: 1}, {k: 2}])
obj.c.first.j #=> 1
obj.c.last.k #=> 2
aetherknight commented 7 years ago

Yeah, your code recurses over arrays and works nicely for a read-only use case, but the feature is optional for ROS (although there is probably a strong case to be made to just make it the default).