Skip to content

Commit 355f41d

Browse files
fcheungjeremy
authored andcommitted
Rework ActiveSupport::OrderedHash to make lookups faster
[rails#1352 state:committed] Signed-off-by: Jeremy Kemper <[email protected]>
1 parent 014b799 commit 355f41d

File tree

3 files changed

+36
-38
lines changed

3 files changed

+36
-38
lines changed

activerecord/test/cases/calculations_test.rb

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -171,8 +171,9 @@ def test_should_group_by_association_with_non_numeric_foreign_key
171171
Account.expects(:columns).at_least_once.returns([column])
172172

173173
c = Account.count(:all, :group => :firm)
174-
assert_equal Firm, c.first.first.class
175-
assert_equal 1, c.first.last
174+
first_key = c.keys.first
175+
assert_equal Firm, first_key.class
176+
assert_equal 1, c[first_key]
176177
end
177178
end
178179

activesupport/lib/active_support/ordered_hash.rb

Lines changed: 23 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -4,62 +4,49 @@ module ActiveSupport
44
if RUBY_VERSION >= '1.9'
55
OrderedHash = ::Hash
66
else
7-
class OrderedHash < Array #:nodoc:
8-
def []=(key, value)
9-
if pair = assoc(key)
10-
pair.pop
11-
pair << value
12-
else
13-
self << [key, value]
14-
end
15-
value
7+
class OrderedHash < Hash #:nodoc:
8+
def initialize(*args, &block)
9+
super
10+
@keys = []
1611
end
1712

18-
def [](key)
19-
pair = assoc(key)
20-
pair ? pair.last : nil
13+
def []=(key, value)
14+
if !has_key?(key)
15+
@keys << key
16+
end
17+
super
2118
end
2219

2320
def delete(key)
24-
pair = assoc(key)
25-
pair ? array_index = index(pair) : nil
26-
array_index ? delete_at(array_index).last : nil
21+
array_index = has_key?(key) && index(key)
22+
if array_index
23+
@keys.delete_at(array_index)
24+
end
25+
super
2726
end
2827

2928
def keys
30-
collect { |key, value| key }
29+
@keys
3130
end
3231

3332
def values
34-
collect { |key, value| value }
33+
@keys.collect { |key| self[key] }
3534
end
3635

3736
def to_hash
38-
returning({}) do |hash|
39-
each { |array| hash[array[0]] = array[1] }
40-
end
41-
end
42-
43-
def has_key?(k)
44-
!assoc(k).nil?
45-
end
46-
47-
alias_method :key?, :has_key?
48-
alias_method :include?, :has_key?
49-
alias_method :member?, :has_key?
50-
51-
def has_value?(v)
52-
any? { |key, value| value == v }
37+
Hash.new(self)
5338
end
5439

55-
alias_method :value?, :has_value?
56-
5740
def each_key
58-
each { |key, value| yield key }
41+
@keys.each { |key| yield key }
5942
end
6043

6144
def each_value
62-
each { |key, value| yield value }
45+
@keys.each { |key| yield self[key]}
46+
end
47+
48+
def each
49+
keys.each {|key| yield [key, self[key]]}
6350
end
6451
end
6552
end

activesupport/test/ordered_hash_test.rb

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,4 +73,14 @@ def test_each_value
7373
@ordered_hash.each_value { |v| values << v }
7474
assert_equal @values, values
7575
end
76+
77+
def test_each
78+
values = []
79+
@ordered_hash.each {|key, value| values << value}
80+
assert_equal @values, values
81+
end
82+
83+
def test_each_with_index
84+
@ordered_hash.each_with_index { |pair, index| assert_equal [@keys[index], @values[index]], pair}
85+
end
7686
end

0 commit comments

Comments
 (0)