Permalink
Browse files

Fix Hash#deep_merge bug and improve documentation — resolves #12738

Previously merging into a hash with a falsy value would not result in
the merge-block being called. The fix is simply to check for presence
of the key in the hash.

The documentation example for `deep_merge` now appropriately
demonstrates what a deep merge does.
  • Loading branch information...
1 parent 4aae538 commit 0438134476b05d14dffbb28f84eac582bd3cfa8f @coreyward coreyward committed Nov 2, 2013
@@ -1,25 +1,35 @@
class Hash
# Returns a new hash with +self+ and +other_hash+ merged recursively.
#
- # h1 = { x: { y: [4, 5, 6] }, z: [7, 8, 9] }
- # h2 = { x: { y: [7, 8, 9] }, z: 'xyz' }
+ # h1 = { a: true, b: { c: [1,2,3] } }
+ # h2 = { a: false, b: { x: [3,4,5] } }
#
- # h1.deep_merge(h2) # => {x: {y: [7, 8, 9]}, z: "xyz"}
- # h2.deep_merge(h1) # => {x: {y: [4, 5, 6]}, z: [7, 8, 9]}
- # h1.deep_merge(h2) { |key, old, new| Array.wrap(old) + Array.wrap(new) }
- # # => {:x=>{:y=>[4, 5, 6, 7, 8, 9]}, :z=>[7, 8, 9, "xyz"]}
+ # h1.deep_merge(h2) #=> { a: false, b: { c: [1,2,3], x: [3,4,5] } }
+ #
+ # Like with Hash#merge in the standard library, a block can be provided
+ # to merge values:
+ #
+ # h1 = { a: 100, b: 200, c: { c1: 100 } }
+ # h2 = { b: 250, c: { c1: 200 } }
+ # h1.deep_merge(h2) { |key, this_val, other_val| this_val + other_val }
+ # # => { a: 100, b: 450, c: { c1: 300 } }
def deep_merge(other_hash, &block)
dup.deep_merge!(other_hash, &block)
end
# Same as +deep_merge+, but modifies +self+.
def deep_merge!(other_hash, &block)
- other_hash.each_pair do |k,v|
- tv = self[k]
- if tv.is_a?(Hash) && v.is_a?(Hash)
- self[k] = tv.deep_merge(v, &block)
+ other_hash.each_pair do |current_key, other_value|
+ this_value = self[current_key]
+
+ self[current_key] = if this_value.is_a?(Hash) && other_value.is_a?(Hash)
+ this_value.deep_merge(other_value, &block)
else
- self[k] = block && tv ? block.call(k, tv, v) : v
+ if block_given? && key?(current_key)
+ block.call(current_key, this_value, other_value)
+ else
+ other_value
+ end
end
end
self
@@ -661,6 +661,16 @@ def test_deep_merge_with_block
assert_equal expected, hash_1
end
+ def test_deep_merge_with_falsey_values
+ hash_1 = { e: false }
+ hash_2 = { e: 'e' }
+ expected = { e: [:e, false, 'e'] }
+ assert_equal(expected, hash_1.deep_merge(hash_2) { |k,o,n| [k, o, n] })
+
+ hash_1.deep_merge!(hash_2) { |k,o,n| [k, o, n] }
+ assert_equal expected, hash_1
+ end
+
def test_deep_merge_on_indifferent_access
hash_1 = HashWithIndifferentAccess.new({ :a => "a", :b => "b", :c => { :c1 => "c1", :c2 => "c2", :c3 => { :d1 => "d1" } } })
hash_2 = HashWithIndifferentAccess.new({ :a => 1, :c => { :c1 => 2, :c3 => { :d2 => "d2" } } })

0 comments on commit 0438134

Please sign in to comment.