Thanks for looking into that Ben. I agree that the bug you found could be
problematic and checking both sides is a good solution.
Also I think it's safer to just check directly for Hash instead of using
> respond_to?(:merge), as it's possible non-Hash objects will have defined a
> "merge" method:
> recursive_merge = lambda{|k, old, new| new === Hash && old === Hash ?
> new.merge(old, &recursive_merge) : new}
>
> This is exactly why I didn't reopen the Hash class, I do want to be able to
merge objects that are "mergeable" even tho they are not Hash instances. One
can subclass Hash for instance and now your code won't work :(
I think this is actually a good candidate for the Y combinator
http://en.wikipedia.org/wiki/Y_combinator and I will give it a try.
- Matt
On Thu, Dec 2, 2010 at 8:40 AM, Ben Hughes <[email protected]> wrote:
> I think that's the best approach, but there's a subtle bug there that might
> catch you depending on which hashes you're dealing with:
>
> What you have works fine when the nesting is identical such as in your
> example, but imagine if instead you have this:
>
> h1 = {:key => {a: 1, b: 2, c: {a: 1, b: 2}}}
> h2 = {:key => {c: {a: 'a', b: {x: 'y'}, c: 'c'}, d: 4}}
>
> (Note the addition of b: {x: 'y'} in the middle, so hash nesting at that
> point in h2 is one more than in h1)
>
> If you do h1.merge(h2, &recursive_merge) it will fail:
> TypeError: can't convert Fixnum into Hash
>
> This is because the lambda is running with "new" as {x: 'y'} (which does
> respond to merge) and "old" as 2, so it's calling {x: 'y'}.merge(2).
> Interestingly the reverse operation (h2.merge(h1)) *would* work but
> naturally has a different result.
>
> So I think to be safe you need to do checking on both old and new:
>
> recursive_merge = lambda{|k, old, new| new.respond_to?(:merge) &&
> old.respond_to?(:merge) ? new.merge(old, &recursive_merge) : new}
>
> Also I think it's safer to just check directly for Hash instead of using
> respond_to?(:merge), as it's possible non-Hash objects will have defined a
> "merge" method:
>
> recursive_merge = lambda{|k, old, new| new === Hash && old === Hash ?
> new.merge(old, &recursive_merge) : new}
>
> It would be really nice if Ruby had some way to reference the defined
> lambda within the block so we didn't have to assign it to a local variable
> for use within the closure, something like:
>
> lambda{|k, old, new| new === Hash && old === Hash ? new.merge(old, &self) :
> new}
>
> But of course self references the object in which the lambda was defined,
> so that doesn't work.
>
>
> On Thu, Dec 2, 2010 at 12:07 AM, Matt Aimonetti
> <[email protected]>wrote:
>
>> Tonight I was trying to do some recursive merging of hashes and the only
>> decent solution I came up with looks like that:
>>
>> h1 = {:key => {a: 1, b: 2, c: {a: 1, b: 2}}}
>> h2 = {:key => {c: {a:'a', c: 'c'}, d: 4}}
>>
>> recursive_merge = lambda{|k, old, new| new.respond_to?(:merge) ?
>> new.merge(old, &recursive_merge) : new}
>> puts h1.merge(h2, &recursive_merge)
>> # => {:key=>{:a=>1, :b=>2, :c=>{:a=>"a", :b=>2, :c=>"c"}, :d=>4}}
>>
>> http://gist.github.com/724900
>>
>> (Yes I'm using Ruby 1.9, and if you aren't yet, I'm sorry for you ;) )
>>
>> The code above seems to be working pretty well but I'm wondering if that's
>> really the best way to do that, what do you guys think and can anyone come
>> up with a better solution?
>>
>> Thanks,
>>
>> - Matt
>>
>> --
>> SD Ruby mailing list
>> [email protected]
>> http://groups.google.com/group/sdruby
>
>
> --
> SD Ruby mailing list
> [email protected]
> http://groups.google.com/group/sdruby
--
SD Ruby mailing list
[email protected]
http://groups.google.com/group/sdruby
