deep_merge overwriting works differently than Hash#merge

[jpb]

deep_merge behaves different than Hash#merge, where Hash#merge overwrites a key's value of the original hash if the hash being merged in contains the same key. The opposite is true for deep_merge.

$ ruby -v
ruby 2.0.0p451 (2014-02-24 revision 45167) [x86_64-darwin13.1.0]
$ irb
irb(main):001:0> require 'deep_merge'
=> true
irb(main):002:0> { key: 'left' }.merge({ key: 'right' })
=> {:key=>"right"}
irb(main):003:0> { key: 'left' }.deep_merge({ key: 'right' })
=> {:key=>"left"}

Is this as intended, or is this a bug in deep_merge?

[jpb]

It looks like there are other inconstancies around nil values (line 6 and 7):

irb(main):001:0> require 'deep_merge'
=> true
irb(main):002:0> {key: 'left'}.merge(key: 'right')
=> {:key=>"right"}
irb(main):003:0> {key: 'left'}.deep_merge(key: 'right')
=> {:key=>"left"}
irb(main):004:0> {key: nil}.merge(key: 'right')
=> {:key=>"right"}
irb(main):005:0> {key: nil}.deep_merge(key: 'right')
=> {:key=>"right"}
irb(main):006:0> {key: 'left'}.merge(key: nil)
=> {:key=>nil}
irb(main):007:0> {key: 'left'}.deep_merge(key: nil)
=> {:key=>"left"}

[science]

@jpb -- I'm the original author of this gem, fyi - I've been away from the coding world for a little while :)

Regarding your first point, it is intentional for deep_merge to deviate from merge. While it's unfortunate in terms of mnemonics / syntax consistency, it's (IMO) preferable b/c we can offer both types of functionality. So you can write this to make it compatible with merge:

{ key: 'left' }.deep_merge!({ key: 'right' })
# => {:key=>"right"}
# Or to prefer the original (no overwriting):
{ key: 'left' }.deep_merge({ key: 'right' })
# => {:key=>"left"}

The inconsistencies in your second example are intentional also. Arguably unhelpful for some use-cases for sure, but the purpose of this gem was originally to merge options coming in from an HTTP request (checkboxes and other filter parameters). A nil option doesn't overwrite existing elements. But if you convert your nil to a "knockout" parameter, you can use it to zero out the existing parameter (different than replacing it with a nil, which may not be a supported operation in the current system). But it gets you closer. It would be possible to fairly easily modify the code to replace an existing with nil, on presence of the knockout parameter. (If you want tips on how to do that let me know). But it's probably harder to change the default behavior of how nils are handled without breaking a bunch of tests (and other people's code dependencies).

{key: 'left'}.deeper_merge!({key: '--'}, {knockout_prefix:'--'})
# => {:key=>""}

Apologies that this library is non-standard in this way. It was originally tailored for a very specific use case and subsequently generalized without consideration for aligning to how Hash.merge works.

[dayglojesus]

😱