From 65d40fc02b75d5a58846d110a3006ccec81730cf Mon Sep 17 00:00:00 2001 From: Marc-Andre Lafortune Date: Wed, 2 Mar 2011 22:59:18 -0500 Subject: [PATCH 1/6] Make tests more precise. --- activesupport/test/core_ext/enumerable_test.rb | 28 ++++++++++++------------ 1 files changed, 14 insertions(+), 14 deletions(-) diff --git a/activesupport/test/core_ext/enumerable_test.rb b/activesupport/test/core_ext/enumerable_test.rb index 4655bfe..dd59898 100644 --- a/activesupport/test/core_ext/enumerable_test.rb +++ b/activesupport/test/core_ext/enumerable_test.rb @@ -29,10 +29,10 @@ class EnumerableTests < Test::Unit::TestCase def test_sums assert_equal 30, [5, 15, 10].sum - assert_equal 30, [5, 15, 10].sum { |i| i } + assert_equal 60, [5, 15, 10].sum { |i| i * 2} assert_equal 'abc', %w(a b c).sum - assert_equal 'abc', %w(a b c).sum { |i| i } + assert_equal 'aabbcc', %w(a b c).sum { |i| i * 2 } payments = [ Payment.new(5), Payment.new(15), Payment.new(10) ] assert_equal 30, payments.sum(&:price) @@ -56,11 +56,11 @@ class EnumerableTests < Test::Unit::TestCase def test_empty_sums assert_equal 0, [].sum - assert_equal 0, [].sum { |i| i } + assert_equal 0, [].sum { |i| i + 10 } assert_equal Payment.new(0), [].sum(Payment.new(0)) end - def test_enumerable_sums + def test_range_sums assert_equal 20, (1..4).sum { |i| i * 2 } assert_equal 10, (1..4).sum assert_equal 10, (1..4.5).sum @@ -80,18 +80,18 @@ class EnumerableTests < Test::Unit::TestCase end def test_many - assert ![].many? - assert ![ 1 ].many? - assert [ 1, 2 ].many? - - assert ![].many? {|x| x > 1 } - assert ![ 2 ].many? {|x| x > 1 } - assert ![ 1, 2 ].many? {|x| x > 1 } - assert [ 1, 2, 2 ].many? {|x| x > 1 } + assert_equal false, [].many? + assert_equal false, [ 1 ].many? + assert_equal true, [ 1, 2 ].many? + + assert_equal false, [].many? {|x| x > 1 } + assert_equal false, [ 2 ].many? {|x| x > 1 } + assert_equal false, [ 1, 2 ].many? {|x| x > 1 } + assert_equal true, [ 1, 2, 2 ].many? {|x| x > 1 } end def test_exclude? - assert [ 1 ].exclude?(2) - assert ![ 1 ].exclude?(1) + assert_equal true, [ 1 ].exclude?(2) + assert_equal false, [ 1 ].exclude?(1) end end -- 1.7.3.3 From 7e8ce42fe72dd51fd09325612fbd8c0d2b02f986 Mon Sep 17 00:00:00 2001 From: Marc-Andre Lafortune Date: Wed, 2 Mar 2011 23:09:56 -0500 Subject: [PATCH 2/6] Test using generic Enumerables instead of arrays. --- activesupport/test/core_ext/enumerable_test.rb | 62 ++++++++++++++--------- 1 files changed, 38 insertions(+), 24 deletions(-) diff --git a/activesupport/test/core_ext/enumerable_test.rb b/activesupport/test/core_ext/enumerable_test.rb index dd59898..9720b06 100644 --- a/activesupport/test/core_ext/enumerable_test.rb +++ b/activesupport/test/core_ext/enumerable_test.rb @@ -8,6 +8,17 @@ class SummablePayment < Payment end class EnumerableTests < Test::Unit::TestCase + class GenericEnumerable + include Enumerable + def initialize(values = [1, 2, 3]) + @values = values + end + + def each + @values.each{|v| yield v} + end + end + def test_group_by names = %w(marcel sam david jeremy) klass = Struct.new(:name) @@ -17,7 +28,7 @@ class EnumerableTests < Test::Unit::TestCase people << p end - grouped = objects.group_by { |object| object.name } + grouped = GenericEnumerable.new(objects).group_by { |object| object.name } grouped.each do |name, group| assert group.all? { |person| person.name == name } @@ -28,17 +39,19 @@ class EnumerableTests < Test::Unit::TestCase end def test_sums - assert_equal 30, [5, 15, 10].sum - assert_equal 60, [5, 15, 10].sum { |i| i * 2} + enum = GenericEnumerable.new([5, 15, 10]) + assert_equal 30, enum.sum + assert_equal 60, enum.sum { |i| i * 2} - assert_equal 'abc', %w(a b c).sum - assert_equal 'aabbcc', %w(a b c).sum { |i| i * 2 } + enum = GenericEnumerable.new(%w(a b c)) + assert_equal 'abc', enum.sum + assert_equal 'aabbcc', enum.sum { |i| i * 2 } - payments = [ Payment.new(5), Payment.new(15), Payment.new(10) ] + payments = GenericEnumerable.new([ Payment.new(5), Payment.new(15), Payment.new(10) ]) assert_equal 30, payments.sum(&:price) assert_equal 60, payments.sum { |p| p.price * 2 } - payments = [ SummablePayment.new(5), SummablePayment.new(15) ] + payments = GenericEnumerable.new([ SummablePayment.new(5), SummablePayment.new(15) ]) assert_equal SummablePayment.new(20), payments.sum assert_equal SummablePayment.new(20), payments.sum { |p| p } end @@ -46,18 +59,18 @@ class EnumerableTests < Test::Unit::TestCase def test_nil_sums expected_raise = TypeError - assert_raise(expected_raise) { [5, 15, nil].sum } + assert_raise(expected_raise) { GenericEnumerable.new([5, 15, nil]).sum } - payments = [ Payment.new(5), Payment.new(15), Payment.new(10), Payment.new(nil) ] + payments = GenericEnumerable.new([ Payment.new(5), Payment.new(15), Payment.new(10), Payment.new(nil) ]) assert_raise(expected_raise) { payments.sum(&:price) } assert_equal 60, payments.sum { |p| p.price.to_i * 2 } end def test_empty_sums - assert_equal 0, [].sum - assert_equal 0, [].sum { |i| i + 10 } - assert_equal Payment.new(0), [].sum(Payment.new(0)) + assert_equal 0, GenericEnumerable.new([]).sum + assert_equal 0, GenericEnumerable.new([]).sum { |i| i + 10 } + assert_equal Payment.new(0), GenericEnumerable.new([]).sum(Payment.new(0)) end def test_range_sums @@ -69,29 +82,30 @@ class EnumerableTests < Test::Unit::TestCase end def test_each_with_object - result = %w(foo bar).each_with_object({}) { |str, hsh| hsh[str] = str.upcase } + enum = GenericEnumerable.new(%w(foo bar)) + result = enum.each_with_object({}) { |str, hsh| hsh[str] = str.upcase } assert_equal({'foo' => 'FOO', 'bar' => 'BAR'}, result) end def test_index_by - payments = [ Payment.new(5), Payment.new(15), Payment.new(10) ] + payments = GenericEnumerable.new([ Payment.new(5), Payment.new(15), Payment.new(10) ]) assert_equal({ 5 => payments[0], 15 => payments[1], 10 => payments[2] }, payments.index_by { |p| p.price }) end def test_many - assert_equal false, [].many? - assert_equal false, [ 1 ].many? - assert_equal true, [ 1, 2 ].many? - - assert_equal false, [].many? {|x| x > 1 } - assert_equal false, [ 2 ].many? {|x| x > 1 } - assert_equal false, [ 1, 2 ].many? {|x| x > 1 } - assert_equal true, [ 1, 2, 2 ].many? {|x| x > 1 } + assert_equal false, GenericEnumerable.new([] ).many? + assert_equal false, GenericEnumerable.new([ 1 ] ).many? + assert_equal true, GenericEnumerable.new([ 1, 2 ] ).many? + + assert_equal false, GenericEnumerable.new([] ).many? {|x| x > 1 } + assert_equal false, GenericEnumerable.new([ 2 ] ).many? {|x| x > 1 } + assert_equal false, GenericEnumerable.new([ 1, 2 ] ).many? {|x| x > 1 } + assert_equal true, GenericEnumerable.new([ 1, 2, 2 ]).many? {|x| x > 1 } end def test_exclude? - assert_equal true, [ 1 ].exclude?(2) - assert_equal false, [ 1 ].exclude?(1) + assert_equal true, GenericEnumerable.new([ 1 ]).exclude?(2) + assert_equal false, GenericEnumerable.new([ 1 ]).exclude?(1) end end -- 1.7.3.3 From ed7e83ae9e07b7526ff04b9eb90d37bbeabc5ff0 Mon Sep 17 00:00:00 2001 From: Marc-Andre Lafortune Date: Wed, 2 Mar 2011 23:19:01 -0500 Subject: [PATCH 3/6] Make Enumerable#many? not rely on #size --- .../lib/active_support/core_ext/enumerable.rb | 4 ++-- activesupport/test/core_ext/enumerable_test.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/activesupport/lib/active_support/core_ext/enumerable.rb b/activesupport/lib/active_support/core_ext/enumerable.rb index 6ecedc2..e3d1692 100644 --- a/activesupport/lib/active_support/core_ext/enumerable.rb +++ b/activesupport/lib/active_support/core_ext/enumerable.rb @@ -93,10 +93,10 @@ module Enumerable Hash[map { |elem| [yield(elem), elem] }] end - # Returns true if the collection has more than 1 element. Functionally equivalent to collection.size > 1. + # Returns true if the enumerable has more than 1 element. Functionally equivalent to enum.to_a.size > 1. # Works with a block too ala any?, so people.many? { |p| p.age > 26 } # => returns true if more than 1 person is over 26. def many?(&block) - size = block_given? ? select(&block).size : self.size + size = block_given? ? select(&block).size : to_a.size size > 1 end diff --git a/activesupport/test/core_ext/enumerable_test.rb b/activesupport/test/core_ext/enumerable_test.rb index 9720b06..688207c 100644 --- a/activesupport/test/core_ext/enumerable_test.rb +++ b/activesupport/test/core_ext/enumerable_test.rb @@ -89,7 +89,7 @@ class EnumerableTests < Test::Unit::TestCase def test_index_by payments = GenericEnumerable.new([ Payment.new(5), Payment.new(15), Payment.new(10) ]) - assert_equal({ 5 => payments[0], 15 => payments[1], 10 => payments[2] }, + assert_equal({ 5 => Payment.new(5), 15 => Payment.new(15), 10 => Payment.new(10) }, payments.index_by { |p| p.price }) end -- 1.7.3.3 From 05a230014cc12919f5d9f209ea889a7593b4e480 Mon Sep 17 00:00:00 2001 From: Marc-Andre Lafortune Date: Wed, 2 Mar 2011 23:29:27 -0500 Subject: [PATCH 4/6] Make Enumerable#many? iterate only over what is necessary --- .../lib/active_support/core_ext/enumerable.rb | 13 ++++++++++--- activesupport/test/core_ext/enumerable_test.rb | 7 +++++++ 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/activesupport/lib/active_support/core_ext/enumerable.rb b/activesupport/lib/active_support/core_ext/enumerable.rb index e3d1692..678e5e8 100644 --- a/activesupport/lib/active_support/core_ext/enumerable.rb +++ b/activesupport/lib/active_support/core_ext/enumerable.rb @@ -95,9 +95,16 @@ module Enumerable # Returns true if the enumerable has more than 1 element. Functionally equivalent to enum.to_a.size > 1. # Works with a block too ala any?, so people.many? { |p| p.age > 26 } # => returns true if more than 1 person is over 26. - def many?(&block) - size = block_given? ? select(&block).size : to_a.size - size > 1 + def many? + cnt = 0 + if block_given? + any? do |element| + cnt += 1 if yield element + cnt > 1 + end + else + any?{ (cnt += 1) > 1 } + end end # The negative of the Enumerable#include?. Returns true if the collection does not include the object. diff --git a/activesupport/test/core_ext/enumerable_test.rb b/activesupport/test/core_ext/enumerable_test.rb index 688207c..98f3f2f 100644 --- a/activesupport/test/core_ext/enumerable_test.rb +++ b/activesupport/test/core_ext/enumerable_test.rb @@ -104,6 +104,13 @@ class EnumerableTests < Test::Unit::TestCase assert_equal true, GenericEnumerable.new([ 1, 2, 2 ]).many? {|x| x > 1 } end + def test_many_iterates_only_on_what_is_needed + infinity = 1.0/0.0 + very_long_enum = 0..infinity + assert_equal true, very_long_enum.many? + assert_equal true, very_long_enum.many?{|x| x > 100} + end + def test_exclude? assert_equal true, GenericEnumerable.new([ 1 ]).exclude?(2) assert_equal false, GenericEnumerable.new([ 1 ]).exclude?(1) -- 1.7.3.3 From cd49512bbeca27e5d3b5949f8a7e6d989df14d33 Mon Sep 17 00:00:00 2001 From: Marc-Andre Lafortune Date: Wed, 2 Mar 2011 23:40:26 -0500 Subject: [PATCH 5/6] Insure that Enumerable#index_by, group_by, ... return Enumerators when no block is given --- .../lib/active_support/core_ext/enumerable.rb | 3 +++ activesupport/test/core_ext/enumerable_test.rb | 13 ++++++++++++- 2 files changed, 15 insertions(+), 1 deletions(-) diff --git a/activesupport/lib/active_support/core_ext/enumerable.rb b/activesupport/lib/active_support/core_ext/enumerable.rb index 678e5e8..be6885f 100644 --- a/activesupport/lib/active_support/core_ext/enumerable.rb +++ b/activesupport/lib/active_support/core_ext/enumerable.rb @@ -20,6 +20,7 @@ module Enumerable # "2006-02-24 -> Transcript, Transcript" # "2006-02-23 -> Transcript" def group_by + return to_enum :group_by unless block_given? assoc = ActiveSupport::OrderedHash.new each do |element| @@ -76,6 +77,7 @@ module Enumerable # (1..5).each_with_object(1) { |value, memo| memo *= value } # => 1 # def each_with_object(memo, &block) + return to_enum :each_with_object, memo unless block_given? each do |element| block.call(element, memo) end @@ -90,6 +92,7 @@ module Enumerable # => { "Chade- Fowlersburg-e" => , "David Heinemeier Hansson" => , ...} # def index_by + return to_enum :index_by unless block_given? Hash[map { |elem| [yield(elem), elem] }] end diff --git a/activesupport/test/core_ext/enumerable_test.rb b/activesupport/test/core_ext/enumerable_test.rb index 98f3f2f..cdfa991 100644 --- a/activesupport/test/core_ext/enumerable_test.rb +++ b/activesupport/test/core_ext/enumerable_test.rb @@ -8,6 +8,8 @@ class SummablePayment < Payment end class EnumerableTests < Test::Unit::TestCase + Enumerator = [].each.class + class GenericEnumerable include Enumerable def initialize(values = [1, 2, 3]) @@ -28,7 +30,8 @@ class EnumerableTests < Test::Unit::TestCase people << p end - grouped = GenericEnumerable.new(objects).group_by { |object| object.name } + enum = GenericEnumerable.new(objects) + grouped = enum.group_by { |object| object.name } grouped.each do |name, group| assert group.all? { |person| person.name == name } @@ -36,6 +39,8 @@ class EnumerableTests < Test::Unit::TestCase assert_equal objects.uniq.map(&:name), grouped.keys assert({}.merge(grouped), "Could not convert ActiveSupport::OrderedHash into Hash") + assert_equal Enumerator, enum.group_by.class + assert_equal grouped, enum.group_by.each(&:name) end def test_sums @@ -85,12 +90,18 @@ class EnumerableTests < Test::Unit::TestCase enum = GenericEnumerable.new(%w(foo bar)) result = enum.each_with_object({}) { |str, hsh| hsh[str] = str.upcase } assert_equal({'foo' => 'FOO', 'bar' => 'BAR'}, result) + assert_equal Enumerator, enum.each_with_object({}).class + result2 = enum.each_with_object({}).each{|str, hsh| hsh[str] = str.upcase} + assert_equal result, result2 end def test_index_by payments = GenericEnumerable.new([ Payment.new(5), Payment.new(15), Payment.new(10) ]) assert_equal({ 5 => Payment.new(5), 15 => Payment.new(15), 10 => Payment.new(10) }, payments.index_by { |p| p.price }) + assert_equal Enumerator, payments.index_by.class + assert_equal({ 5 => Payment.new(5), 15 => Payment.new(15), 10 => Payment.new(10) }, + payments.index_by.each { |p| p.price }) end def test_many -- 1.7.3.3 From 553ca697d52e516fd1ee7206f4d8e8b9c3f6b59e Mon Sep 17 00:00:00 2001 From: Marc-Andre Lafortune Date: Wed, 2 Mar 2011 23:47:35 -0500 Subject: [PATCH 6/6] Trivial optimization for Enumerable#each_with_object --- .../lib/active_support/core_ext/enumerable.rb | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/activesupport/lib/active_support/core_ext/enumerable.rb b/activesupport/lib/active_support/core_ext/enumerable.rb index be6885f..588cf6b 100644 --- a/activesupport/lib/active_support/core_ext/enumerable.rb +++ b/activesupport/lib/active_support/core_ext/enumerable.rb @@ -76,10 +76,10 @@ module Enumerable # # (1..5).each_with_object(1) { |value, memo| memo *= value } # => 1 # - def each_with_object(memo, &block) + def each_with_object(memo) return to_enum :each_with_object, memo unless block_given? each do |element| - block.call(element, memo) + yield element, memo end memo end unless [].respond_to?(:each_with_object) -- 1.7.3.3