From b4167d3f3e4d25be16e06e71afd1c64a47ca54d7 Mon Sep 17 00:00:00 2001 From: Alexey Vakhov Date: Fri, 25 May 2012 17:46:53 +0400 Subject: Fix Range#sum optimized version At 1bd4d1c67459a91415ee73a8f55d2309c0d62a87 was added Range#sum optimized version for arithmetic progressions. This improvment injected a defect with not integer range boundaries. The defect was fixed by e0adfa82c05f9c975005f102b4bcaebfcd17d241. The second commit really disabled optimization at all because in Ruby integer-valued numbers are instances of Fixnum and Bignum classes. We should #use is_a? (#kind_of?) method instead #instance_of? to check if value is numerical: 1.class # => Fixnum 1.instance_of?(Integer) # => false 1.is_a?(Integer) # => true -100_000_000_000.class # => Bignum -100_000_000_000.instance_of?(Integer) # => false -100_000_000_000.is_a?(Integer) # => true Moreover original implementation of Range#sum has a defect with reverse range boundaries. If the first boundary is less than the second range is empty. Current commit fixes and tests this case too. --- activesupport/lib/active_support/core_ext/enumerable.rb | 8 ++++++-- activesupport/test/core_ext/enumerable_test.rb | 5 +++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/activesupport/lib/active_support/core_ext/enumerable.rb b/activesupport/lib/active_support/core_ext/enumerable.rb index 02d5a7080f..03efe6a19a 100644 --- a/activesupport/lib/active_support/core_ext/enumerable.rb +++ b/activesupport/lib/active_support/core_ext/enumerable.rb @@ -65,11 +65,15 @@ class Range #:nodoc: # Optimize range sum to use arithmetic progression if a block is not given and # we have a range of numeric values. def sum(identity = 0) - if block_given? || !(first.instance_of?(Integer) && last.instance_of?(Integer)) + if block_given? || !(first.is_a?(Integer) && last.is_a?(Integer)) super else actual_last = exclude_end? ? (last - 1) : last - (actual_last - first + 1) * (actual_last + first) / 2 + if actual_last >= first + (actual_last - first + 1) * (actual_last + first) / 2 + else + identity + end end end end diff --git a/activesupport/test/core_ext/enumerable_test.rb b/activesupport/test/core_ext/enumerable_test.rb index 0bf48dd378..0a1abac767 100644 --- a/activesupport/test/core_ext/enumerable_test.rb +++ b/activesupport/test/core_ext/enumerable_test.rb @@ -84,6 +84,11 @@ class EnumerableTests < ActiveSupport::TestCase assert_equal 10, (1..4.5).sum assert_equal 6, (1...4).sum 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) + assert_equal 10, (10..10).sum + assert_equal 42, (10...10).sum(42) end def test_index_by -- cgit v1.2.3