Rails Lighthouse ticket #6510

enumerable_patch.diff

From 65d40fc02b75d5a58846d110a3006ccec81730cf Mon Sep 17 00:00:00 2001
From: Marc-Andre Lafortune <github@marc-andre.ca>
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 <github@marc-andre.ca>
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 <github@marc-andre.ca>
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 <github@marc-andre.ca>
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 <github@marc-andre.ca>
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" => <Person ...>, "David Heinemeier Hansson" => <Person ...>, ...}
   #
   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 <github@marc-andre.ca>
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