Permalink
Browse files

Make delete_if/reject faster and fix other mutators

[#1559 state:committed]

Signed-off-by: Jeremy Kemper <jeremy@bitsweat.net>
  • Loading branch information...
1 parent 49306cc commit 4dcd8f01afe5800baa67bbdf72832afb0d627755 @fcheung fcheung committed with dhh Dec 11, 2008
Showing with 61 additions and 13 deletions.
  1. +24 −13 activesupport/lib/active_support/ordered_hash.rb
  2. +37 −0 activesupport/test/ordered_hash_test.rb
@@ -18,19 +18,13 @@ def []=(key, value)
end
def delete(key)
- array_index = has_key?(key) && index(key)
- if array_index
- @keys.delete_at(array_index)
+ if has_key? key
+ index = @keys.index(key)
+ @keys.delete_at index
end
super
end
- def delete_if
- super
- sync_keys!
- self
- end
-
def reject!
super
sync_keys!
@@ -41,9 +35,6 @@ def reject(&block)
dup.reject!(&block)
end
- alias_method :super_keys, :keys
- private :super_keys
-
def keys
@keys
end
@@ -68,10 +59,30 @@ def each
keys.each {|key| yield [key, self[key]]}
end
+ alias_method :each_pair, :each
+
+ def clear
+ super
+ @keys.clear
+ self
+ end
+
+ def shift
+ k = @keys.first
+ v = delete(k)
+ [k, v]
+ end
+
+ def merge(other_hash)
+ result = dup
+ other_hash.each {|k,v| result[k]=v}
+ result
+ end
+
private
def sync_keys!
- (@keys - super_keys).each { |k| @keys.delete(k) }
+ @keys.delete_if {|k| !has_key?(k)}
end
end
end
@@ -36,9 +36,11 @@ def test_delete
@ordered_hash[key] = value
assert_equal @keys.length + 1, @ordered_hash.length
+ assert_equal @ordered_hash.keys.length, @ordered_hash.length
assert_equal value, @ordered_hash.delete(key)
assert_equal @keys.length, @ordered_hash.length
+ assert_equal @ordered_hash.keys.length, @ordered_hash.length
assert_nil @ordered_hash.delete(bad_key)
end
@@ -84,6 +86,17 @@ def test_each_with_index
@ordered_hash.each_with_index { |pair, index| assert_equal [@keys[index], @values[index]], pair}
end
+ def test_each_pair
+ values = []
+ keys = []
+ @ordered_hash.each_pair do |key, value|
+ keys << key
+ values << value
+ end
+ assert_equal @values, values
+ assert_equal @keys, keys
+ end
+
def test_delete_if
(copy = @ordered_hash.dup).delete('pink')
assert_equal copy, @ordered_hash.delete_if { |k, _| k == 'pink' }
@@ -103,4 +116,28 @@ def test_reject
assert_equal copy, @ordered_hash
assert !new_ordered_hash.keys.include?('pink')
end
+
+ def test_clear
+ @ordered_hash.clear
+ assert_equal [], @ordered_hash.keys
+ end
+
+ def test_merge
+ other_hash = ActiveSupport::OrderedHash.new
+ other_hash['purple'] = '800080'
+ other_hash['violet'] = 'ee82ee'
+ merged = @ordered_hash.merge other_hash
+ assert_equal merged.length, @ordered_hash.length + other_hash.length
+ assert_equal @keys + ['purple', 'violet'], merged.keys
+
+ @ordered_hash.merge! other_hash
+ assert_equal @ordered_hash, merged
+ assert_equal @ordered_hash.keys, merged.keys
+ end
+
+ def test_shift
+ pair = @ordered_hash.shift
+ assert_equal [@keys.first, @values.first], pair
+ assert !@ordered_hash.keys.include?(pair.first)
+ end
end

0 comments on commit 4dcd8f0

Please sign in to comment.