Skip to content

Commit

Permalink
Merge pull request rails#42080 from alberto-mota/remove_sum
Browse files Browse the repository at this point in the history
Deprecates `Enumerable#sum` and `Array#sum`
  • Loading branch information
pixeltrix authored May 8, 2021
2 parents 523a526 + afa8335 commit 1fd8a3f
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 14 deletions.
2 changes: 1 addition & 1 deletion activerecord/test/models/invoice.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@
class Invoice < ActiveRecord::Base
has_many :line_items, autosave: true
has_many :shipping_lines, -> { from("shipping_lines") }, autosave: true
before_save { |record| record.balance = record.line_items.map(&:amount).sum }
before_save { |record| record.balance = record.line_items.map(&:amount).compact.sum }
end
15 changes: 15 additions & 0 deletions activesupport/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,21 @@

*Andrew White*

* Deprecates Rails custom `Enumerable#sum` and `Array#sum` in favor of Ruby's native implementation which
is considerably faster.

Ruby requires an initializer for non-numeric type as per examples below:

```ruby
%w[foo bar].sum('')
# instead of %w[foo bar].sum
[[1, 2], [3, 4, 5]].sum([])
#instead of [[1, 2], [3, 4, 5]].sum
```

*Alberto Mota*

* Tests parallelization is now disabled when running individual files to prevent the setup overhead.

It can still be enforced if the environment variable `PARALLEL_WORKERS` is present and set to a value greater than 1.
Expand Down
18 changes: 14 additions & 4 deletions activesupport/lib/active_support/core_ext/enumerable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ def maximum(key)
# It can also calculate the sum without the use of a block.
#
# [5, 15, 10].sum # => 30
# ['foo', 'bar'].sum # => "foobar"
# [[1, 2], [3, 1, 5]].sum # => [1, 2, 3, 1, 5]
# ['foo', 'bar'].sum('') # => "foobar"
# [[1, 2], [3, 1, 5]].sum([]) # => [1, 2, 3, 1, 5]
#
# The default sum of an empty list is zero. You can override this default:
#
Expand All @@ -58,8 +58,19 @@ def sum(identity = nil, &block)
if identity
_original_sum_with_required_identity(identity, &block)
elsif block_given?
map(&block).sum(identity)
map(&block).sum
# we check `first(1) == []` to check if we have an
# empty Enumerable; checking `empty?` would return
# true for `[nil]`, which we want to deprecate to
# keep consistent with Ruby
elsif first.is_a?(Numeric) || first(1) == []
identity ||= 0
_original_sum_with_required_identity(identity, &block)
else
ActiveSupport::Deprecation.warn(<<-MSG.squish)
Rails 7.0 has deprecated Enumerable.sum in favor of Ruby's native implementation available since 2.4.
Sum of non-numeric elements requires an initial argument.
MSG
inject(:+) || 0
end
end
Expand Down Expand Up @@ -279,7 +290,6 @@ def sum(identity = nil)
}

class Array #:nodoc:
# Array#sum was added in Ruby 2.4 but it only works with Numeric elements.
def sum(init = nil, &block)
if init.is_a?(Numeric) || first.is_a?(Numeric)
init ||= 0
Expand Down
50 changes: 41 additions & 9 deletions activesupport/test/core_ext/enumerable_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,16 +55,28 @@ def test_sums
assert_equal 60, enum.sum { |i| i * 2 }

enum = GenericEnumerable.new(%w(a b c))
assert_equal "abc", enum.sum
assert_equal "aabbcc", enum.sum { |i| i * 2 }
assert_equal "abc", enum.sum("")
assert_equal "aabbcc", enum.sum("") { |i| i * 2 }
assert_deprecated do
assert_equal "abc", enum.sum
end
assert_deprecated do
assert_equal "aabbcc", enum.sum { |i| i * 2 }
end

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 = GenericEnumerable.new([ SummablePayment.new(5), SummablePayment.new(15) ])
assert_equal SummablePayment.new(20), payments.sum
assert_equal SummablePayment.new(20), payments.sum { |p| p }
assert_deprecated do
assert_equal SummablePayment.new(20), payments.sum
end
assert_equal SummablePayment.new(20), payments.sum(SummablePayment.new(0))
assert_equal SummablePayment.new(20), payments.sum(SummablePayment.new(0)) { |p| p }
assert_deprecated do
assert_equal SummablePayment.new(20), payments.sum { |p| p }
end

sum = GenericEnumerable.new([3, 5.quo(1)]).sum
assert_typed_equal(8, sum, Rational)
Expand Down Expand Up @@ -105,6 +117,9 @@ def test_nil_sums
expected_raise = TypeError

assert_raise(expected_raise) { GenericEnumerable.new([5, 15, nil]).sum }
assert_deprecated do
assert_equal 0, [nil].sum
end

payments = GenericEnumerable.new([ Payment.new(5), Payment.new(15), Payment.new(10), Payment.new(nil) ])
assert_raise(expected_raise) { payments.sum(&:price) }
Expand All @@ -114,7 +129,9 @@ def test_nil_sums

def test_empty_sums
assert_equal 0, GenericEnumerable.new([]).sum
assert_equal [], GenericEnumerable.new([]).sum([])
assert_equal 0, GenericEnumerable.new([]).sum { |i| i + 10 }
assert_equal [], GenericEnumerable.new([]).sum([]) { |i| i + 10 }
assert_equal Payment.new(0), GenericEnumerable.new([]).sum(Payment.new(0))
assert_typed_equal 0.0, GenericEnumerable.new([]).sum(0.0), Float
end
Expand All @@ -124,7 +141,10 @@ def test_range_sums
assert_equal 10, (1..4).sum
assert_equal 10, (1..4.5).sum
assert_equal 6, (1...4).sum
assert_equal "abc", ("a".."c").sum
assert_deprecated do
assert_equal "abc", ("a".."c").sum
end
assert_equal "abc", ("a".."c").sum("")
assert_equal 50_000_005_000_000, (0..10_000_000).sum
assert_equal 0, (10..0).sum
assert_equal 5, (10..0).sum(5)
Expand All @@ -142,16 +162,28 @@ def test_array_sums
assert_equal 60, enum.sum { |i| i * 2 }

enum = %w(a b c)
assert_equal "abc", enum.sum
assert_equal "aabbcc", enum.sum { |i| i * 2 }
assert_deprecated do
assert_equal "abc", enum.sum
end
assert_equal "abc", enum.sum("")
assert_deprecated do
assert_equal "aabbcc", enum.sum { |i| i * 2 }
end
assert_equal "aabbcc", enum.sum("") { |i| i * 2 }

payments = [ 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) ]
assert_equal SummablePayment.new(20), payments.sum
assert_equal SummablePayment.new(20), payments.sum { |p| p }
assert_deprecated do
assert_equal SummablePayment.new(20), payments.sum
end
assert_equal SummablePayment.new(20), payments.sum(SummablePayment.new(0))
assert_deprecated do
assert_equal SummablePayment.new(20), payments.sum { |p| p }
end
assert_equal SummablePayment.new(20), payments.sum(SummablePayment.new(0)) { |p| p }

sum = [3, 5.quo(1)].sum
assert_typed_equal(8, sum, Rational)
Expand Down

0 comments on commit 1fd8a3f

Please sign in to comment.