Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Optimize Range#sum to use arithmetic progression when a block is not …

…given [#2489].

Signed-off-by: Pratik Naik <pratiknaik@gmail.com>
  • Loading branch information...
commit 1bd4d1c67459a91415ee73a8f55d2309c0d62a87 1 parent d811864
José Valim josevalim authored lifo committed
9 activesupport/lib/active_support/core_ext/enumerable.rb
View
@@ -111,3 +111,12 @@ def none?(&block)
!any?(&block)
end unless [].respond_to?(:none?)
end
+
+class Range #:nodoc:
+ # Optimize range sum to use arithmetic progression if a block is not given.
+ def sum(identity=0, &block)
+ return super if block_given?
+ actual_last = exclude_end? ? (last - 1) : last
+ (actual_last - first + 1) * (actual_last + first) / 2
+ end
+end
2  activesupport/test/core_ext/enumerable_test.rb
View
@@ -61,7 +61,9 @@ def test_empty_sums
end
def test_enumerable_sums
+ assert_equal 20, (1..4).sum { |i| i * 2 }
assert_equal 10, (1..4).sum
+ assert_equal 6, (1...4).sum
end
def test_each_with_object

3 comments on commit 1bd4d1c

Marc-André Lafortune

You should not assume that a Range is a range of fixnums!

For example:
('a'..'c').sum now fails

You should return super if block_given? || !(first.instance_of?(Fixnum) || last.instance_of?(Fixnum))

Joshua Clayton

Marc-Andre,

It could be argued that you should know not to call sum on a range of characters, much like how you wouldn't call upcase or strip on a fixnum. However, I think it makes more sense to concatenate the characters together, much like what Array#sum does => ("a".."c").map.sum == "abc"

José Valim
Owner

I was going to ask why someone would sum something which is not numbers, but @joshuaclayton gave a good example. :)
Already provided a patch, thanks guys.

Please sign in to comment.
Something went wrong with that request. Please try again.