Merge branch 'number-helpers-refactor'

Refactor and improve a bit number helpers code.

5 files changed, 159 insertions, 190 deletions

diff --git a/actionpack/lib/action_view/helpers/number_helper.rb b/actionpack/lib/action_view/helpers/number_helper.rb
index 8f97d1f014..9720e90429 100644
--- a/actionpack/lib/action_view/helpers/number_helper.rb
+++ b/actionpack/lib/action_view/helpers/number_helper.rb
@@ -59,7 +59,7 @@ module ActionView
         return unless number
         options = options.symbolize_keys
 
-        parse_float(number, true) if options[:raise]
+        parse_float(number, true) if options.delete(:raise)
         ERB::Util.html_escape(ActiveSupport::NumberHelper.number_to_phone(number, options))
       end
 
@@ -109,7 +109,9 @@ module ActionView
         return unless number
         options = escape_unsafe_delimiters_and_separators(options.symbolize_keys)
 
-        wrap_with_output_safety_handling(number, options[:raise]){ ActiveSupport::NumberHelper.number_to_currency(number, options) }
+        wrap_with_output_safety_handling(number, options.delete(:raise)) {
+          ActiveSupport::NumberHelper.number_to_currency(number, options)
+        }
       end
 
       # Formats a +number+ as a percentage string (e.g., 65%). You can
@@ -152,7 +154,9 @@ module ActionView
         return unless number
         options = escape_unsafe_delimiters_and_separators(options.symbolize_keys)
 
-        wrap_with_output_safety_handling(number, options[:raise]){ ActiveSupport::NumberHelper.number_to_percentage(number, options) }
+        wrap_with_output_safety_handling(number, options.delete(:raise)) {
+          ActiveSupport::NumberHelper.number_to_percentage(number, options)
+        }
       end
 
       # Formats a +number+ with grouped thousands using +delimiter+
@@ -187,7 +191,9 @@ module ActionView
       def number_with_delimiter(number, options = {})
         options = escape_unsafe_delimiters_and_separators(options.symbolize_keys)
 
-        wrap_with_output_safety_handling(number, options[:raise]){ ActiveSupport::NumberHelper.number_to_delimited(number, options) }
+        wrap_with_output_safety_handling(number, options.delete(:raise)) {
+          ActiveSupport::NumberHelper.number_to_delimited(number, options)
+        }
       end
 
       # Formats a +number+ with the specified level of
@@ -234,7 +240,9 @@ module ActionView
       def number_with_precision(number, options = {})
         options = escape_unsafe_delimiters_and_separators(options.symbolize_keys)
 
-        wrap_with_output_safety_handling(number, options[:raise]){ ActiveSupport::NumberHelper.number_to_rounded(number, options) }
+        wrap_with_output_safety_handling(number, options.delete(:raise)) {
+          ActiveSupport::NumberHelper.number_to_rounded(number, options)
+        }
       end
 
 
@@ -288,7 +296,9 @@ module ActionView
       def number_to_human_size(number, options = {})
         options = escape_unsafe_delimiters_and_separators(options.symbolize_keys)
 
-        wrap_with_output_safety_handling(number, options[:raise]){ ActiveSupport::NumberHelper.number_to_human_size(number, options) }
+        wrap_with_output_safety_handling(number, options.delete(:raise)) {
+          ActiveSupport::NumberHelper.number_to_human_size(number, options)
+        }
       end
 
       # Pretty prints (formats and approximates) a number in a way it
@@ -392,7 +402,9 @@ module ActionView
       def number_to_human(number, options = {})
         options = escape_unsafe_delimiters_and_separators(options.symbolize_keys)
 
-        wrap_with_output_safety_handling(number, options[:raise]){ ActiveSupport::NumberHelper.number_to_human(number, options) }
+        wrap_with_output_safety_handling(number, options.delete(:raise)) {
+          ActiveSupport::NumberHelper.number_to_human(number, options)
+        }
       end
 
       private
diff --git a/actionpack/test/template/number_helper_i18n_test.rb b/actionpack/test/template/number_helper_i18n_test.rb
deleted file mode 100644
index d6e9de9555..0000000000
--- a/actionpack/test/template/number_helper_i18n_test.rb
+++ /dev/null
@@ -1,122 +0,0 @@
-require 'abstract_unit'
-
-class NumberHelperTest < ActionView::TestCase
-  tests ActionView::Helpers::NumberHelper
-
-  def setup
-    I18n.backend.store_translations 'ts',
-      :number => {
-        :format => { :precision => 3, :delimiter => ',', :separator => '.', :significant => false, :strip_insignificant_zeros => false },
-        :currency => { :format => { :unit => '&$', :format => '%u - %n', :negative_format => '(%u - %n)', :precision => 2 } },
-        :human => {
-          :format => {
-            :precision => 2,
-            :significant => true,
-            :strip_insignificant_zeros => true
-          },
-          :storage_units => {
-            :format => "%n %u",
-            :units => {
-              :byte => "b",
-              :kb => "k"
-            }
-          },
-          :decimal_units => {
-            :format => "%n %u",
-            :units => {
-              :deci => {:one => "Tenth", :other => "Tenths"},
-              :unit =>  "u",
-              :ten => {:one => "Ten", :other => "Tens"},
-              :thousand => "t",
-              :million => "m" ,
-              :billion =>"b" ,
-              :trillion =>"t" ,
-              :quadrillion =>"q"
-            }
-          }
-        },
-        :percentage => { :format => {:delimiter => '', :precision => 2, :strip_insignificant_zeros => true} },
-        :precision => { :format => {:delimiter => '', :significant => true} }
-      },
-      :custom_units_for_number_to_human => {:mili => "mm", :centi => "cm", :deci => "dm", :unit => "m", :ten => "dam", :hundred => "hm", :thousand => "km"}
-  end
-
-  def test_number_to_i18n_currency
-    assert_equal("&$ - 10.00", number_to_currency(10, :locale => 'ts'))
-    assert_equal("(&$ - 10.00)", number_to_currency(-10, :locale => 'ts'))
-    assert_equal("-10.00 - &$", number_to_currency(-10, :locale => 'ts', :format => "%n - %u"))
-  end
-
-  def test_number_to_currency_with_clean_i18n_settings
-    clean_i18n do
-      assert_equal("$10.00", number_to_currency(10))
-      assert_equal("-$10.00", number_to_currency(-10))
-    end
-  end
-  
-  def test_number_to_currency_without_currency_negative_format
-    clean_i18n do
-      I18n.backend.store_translations 'ts', :number => { :currency => { :format => { :unit => '@', :format => '%n %u' } } }
-      assert_equal("-10.00 @", number_to_currency(-10, :locale => 'ts'))
-    end
-  end
-
-  def test_number_with_i18n_precision
-    #Delimiter was set to ""
-    assert_equal("10000", number_with_precision(10000, :locale => 'ts'))
-
-    #Precision inherited and significant was set
-    assert_equal("1.00", number_with_precision(1.0, :locale => 'ts'))
-
-  end
-
-  def test_number_with_i18n_delimiter
-    #Delimiter "," and separator "."
-    assert_equal("1,000,000.234", number_with_delimiter(1000000.234, :locale => 'ts'))
-  end
-
-  def test_number_to_i18n_percentage
-    # to see if strip_insignificant_zeros is true
-    assert_equal("1%", number_to_percentage(1, :locale => 'ts'))
-    # precision is 2, significant should be inherited
-    assert_equal("1.24%", number_to_percentage(1.2434, :locale => 'ts'))
-    # no delimiter
-    assert_equal("12434%", number_to_percentage(12434, :locale => 'ts'))
-  end
-
-  def test_number_to_i18n_human_size
-    #b for bytes and k for kbytes
-    assert_equal("2 k", number_to_human_size(2048, :locale => 'ts'))
-    assert_equal("42 b", number_to_human_size(42, :locale => 'ts'))
-  end
-
-  def test_number_to_human_with_default_translation_scope
-    #Using t for thousand
-    assert_equal "2 t", number_to_human(2000, :locale => 'ts')
-    #Significant was set to true with precision 2, using b for billion
-    assert_equal "1.2 b", number_to_human(1234567890, :locale => 'ts')
-    #Using pluralization (Ten/Tens and Tenth/Tenths)
-    assert_equal "1 Tenth", number_to_human(0.1, :locale => 'ts')
-    assert_equal "1.3 Tenth", number_to_human(0.134, :locale => 'ts')
-    assert_equal "2 Tenths", number_to_human(0.2, :locale => 'ts')
-    assert_equal "1 Ten", number_to_human(10, :locale => 'ts')
-    assert_equal "1.2 Ten", number_to_human(12, :locale => 'ts')
-    assert_equal "2 Tens", number_to_human(20, :locale => 'ts')
-  end
-
-  def test_number_to_human_with_custom_translation_scope
-    #Significant was set to true with precision 2, with custom translated units
-    assert_equal "4.3 cm", number_to_human(0.0432, :locale => 'ts', :units => :custom_units_for_number_to_human)
-  end
-
-  private
-    def clean_i18n
-      load_path = I18n.load_path.dup
-      I18n.load_path.clear
-      I18n.reload!
-      yield
-    ensure
-      I18n.load_path = load_path
-      I18n.reload!
-    end
-end
diff --git a/actionpack/test/template/number_helper_test.rb b/actionpack/test/template/number_helper_test.rb
index 057cb47f53..d8fffe75ed 100644
--- a/actionpack/test/template/number_helper_test.rb
+++ b/actionpack/test/template/number_helper_test.rb
@@ -361,69 +361,39 @@ class NumberHelperTest < ActionView::TestCase
   end
 
   def test_number_helpers_should_raise_error_if_invalid_when_specified
-    assert_raise InvalidNumberError do
+    exception = assert_raise InvalidNumberError do
       number_to_human("x", :raise => true)
     end
-    begin
-      number_to_human("x", :raise => true)
-    rescue InvalidNumberError => e
-      assert_equal "x", e.number
-    end
+    assert_equal "x", exception.number
 
-    assert_raise InvalidNumberError do
-      number_to_human_size("x", :raise => true)
-    end
-    begin
+    exception = assert_raise InvalidNumberError do
       number_to_human_size("x", :raise => true)
-    rescue InvalidNumberError => e
-      assert_equal "x", e.number
     end
+    assert_equal "x", exception.number
 
-    assert_raise InvalidNumberError do
+    exception = assert_raise InvalidNumberError do
       number_with_precision("x", :raise => true)
     end
-    begin
-      number_with_precision("x", :raise => true)
-    rescue InvalidNumberError => e
-      assert_equal "x", e.number
-    end
+    assert_equal "x", exception.number
 
-    assert_raise InvalidNumberError do
+    exception = assert_raise InvalidNumberError do
       number_to_currency("x", :raise => true)
     end
-    begin
-      number_with_precision("x", :raise => true)
-    rescue InvalidNumberError => e
-      assert_equal "x", e.number
-    end
+    assert_equal "x", exception.number
 
-    assert_raise InvalidNumberError do
+    exception = assert_raise InvalidNumberError do
       number_to_percentage("x", :raise => true)
     end
-    begin
-      number_to_percentage("x", :raise => true)
-    rescue InvalidNumberError => e
-      assert_equal "x", e.number
-    end
+    assert_equal "x", exception.number
 
-    assert_raise InvalidNumberError do
-      number_with_delimiter("x", :raise => true)
-    end
-    begin
+    exception = assert_raise InvalidNumberError do
       number_with_delimiter("x", :raise => true)
-    rescue InvalidNumberError => e
-      assert_equal "x", e.number
     end
+    assert_equal "x", exception.number
 
-    assert_raise InvalidNumberError do
+    exception = assert_raise InvalidNumberError do
       number_to_phone("x", :raise => true)
     end
-    begin
-      number_to_phone("x", :raise => true)
-    rescue InvalidNumberError => e
-      assert_equal "x", e.number
-    end
-
+    assert_equal "x", exception.number
   end
-
 end
diff --git a/activesupport/lib/active_support/number_helper.rb b/activesupport/lib/active_support/number_helper.rb
index c736041066..99f6489adb 100644
--- a/activesupport/lib/active_support/number_helper.rb
+++ b/activesupport/lib/active_support/number_helper.rb
@@ -7,9 +7,14 @@ module ActiveSupport
   module NumberHelper
     extend self
 
+    DECIMAL_UNITS = { 0 => :unit, 1 => :ten, 2 => :hundred, 3 => :thousand, 6 => :million, 9 => :billion, 12 => :trillion, 15 => :quadrillion,
+      -1 => :deci, -2 => :centi, -3 => :mili, -6 => :micro, -9 => :nano, -12 => :pico, -15 => :femto }
+
     DEFAULT_CURRENCY_VALUES = { :format => "%u%n", :negative_format => "-%u%n", :unit => "$", :separator => ".", :delimiter => ",",
                                 :precision => 2, :significant => false, :strip_insignificant_zeros => false }
 
+    STORAGE_UNITS = [:byte, :kb, :mb, :gb, :tb]
+
     # Formats a +number+ into a US phone number (e.g., (555)
     # 123-9876). You can customize the format in the +options+ hash.
     #
@@ -116,8 +121,7 @@ module ActiveSupport
         number = number.respond_to?("abs") ? number.abs : number.sub(/^-/, '')
       end
 
-      formatted_number = format.gsub('%n', self.number_to_rounded(number, options)).gsub('%u', unit)
-      formatted_number
+      format.gsub('%n', self.number_to_rounded(number, options)).gsub('%u', unit)
     end
 
     # Formats a +number+ as a percentage string (e.g., 65%). You can
@@ -160,9 +164,7 @@ module ActiveSupport
       options  = defaults.merge!(options)
 
       format = options[:format] || "%n%"
-
-      formatted_number = format.gsub('%n', self.number_to_rounded(number, options))
-      formatted_number
+      format.gsub('%n', self.number_to_rounded(number, options))
     end
 
     # Formats a +number+ with grouped thousands using +delimiter+
@@ -242,10 +244,9 @@ module ActiveSupport
     #   number_to_rounded(1111.2345, precision: 2, separator: ',', delimiter: '.')
     #   # => 1.111,23
     def number_to_rounded(number, options = {})
-      options = options.symbolize_keys
-
       return number unless valid_float?(number)
-      number = Float(number)
+      number  = Float(number)
+      options = options.symbolize_keys
 
       defaults = format_translations('precision', options[:locale])
       options  = defaults.merge!(options)
@@ -254,7 +255,7 @@ module ActiveSupport
       significant = options.delete :significant
       strip_insignificant_zeros = options.delete :strip_insignificant_zeros
 
-      if significant and precision > 0
+      if significant && precision > 0
         if number == 0
           digits, rounded_number = 1, 0
         else
@@ -263,10 +264,10 @@ module ActiveSupport
           digits = (Math.log10(rounded_number.abs) + 1).floor # After rounding, the number of digits may have changed
         end
         precision -= digits
-        precision = precision > 0 ? precision : 0  #don't let it be negative
+        precision = 0 if precision < 0 # don't let it be negative
       else
         rounded_number = BigDecimal.new(number.to_s).round(precision).to_f
-        rounded_number = rounded_number.zero? ? rounded_number.abs : rounded_number #prevent showing negative zeros
+        rounded_number = rounded_number.abs if rounded_number.zero? # prevent showing negative zeros
       end
       formatted_number = self.number_to_delimited("%01.#{precision}f" % rounded_number, options)
       if strip_insignificant_zeros
@@ -277,8 +278,6 @@ module ActiveSupport
       end
     end
 
-    STORAGE_UNITS = [:byte, :kb, :mb, :gb, :tb].freeze
-
     # Formats the bytes in +number+ into a more understandable
     # representation (e.g., giving it 1500 yields 1.5 KB). This
     # method is useful for reporting file sizes to users. You can
@@ -356,9 +355,6 @@ module ActiveSupport
       end
     end
 
-    DECIMAL_UNITS = {0 => :unit, 1 => :ten, 2 => :hundred, 3 => :thousand, 6 => :million, 9 => :billion, 12 => :trillion, 15 => :quadrillion,
-      -1 => :deci, -2 => :centi, -3 => :mili, -6 => :micro, -9 => :nano, -12 => :pico, -15 => :femto}.freeze
-
     # Pretty prints (formats and approximates) a number in a way it
     # is more readable by humans (eg.: 1200000000 becomes "1.2
     # Billion"). This is useful for numbers that can get very large
@@ -527,6 +523,5 @@ module ActiveSupport
       false
     end
     private_module_and_instance_method :valid_float?
-
   end
 end
diff --git a/activesupport/test/number_helper_i18n_test.rb b/activesupport/test/number_helper_i18n_test.rb
new file mode 100644
index 0000000000..e07198027b
--- /dev/null
+++ b/activesupport/test/number_helper_i18n_test.rb
@@ -0,0 +1,114 @@
+require 'abstract_unit'
+require 'active_support/number_helper'
+
+module ActiveSupport
+  class NumberHelperI18nTest < ActiveSupport::TestCase
+    include ActiveSupport::NumberHelper
+
+    def setup
+      I18n.backend.store_translations 'ts',
+        :number => {
+        :format => { :precision => 3, :delimiter => ',', :separator => '.', :significant => false, :strip_insignificant_zeros => false },
+        :currency => { :format => { :unit => '&$', :format => '%u - %n', :negative_format => '(%u - %n)', :precision => 2 } },
+        :human => {
+          :format => {
+            :precision => 2,
+            :significant => true,
+            :strip_insignificant_zeros => true
+          },
+          :storage_units => {
+            :format => "%n %u",
+            :units => {
+              :byte => "b",
+              :kb => "k"
+            }
+          },
+          :decimal_units => {
+            :format => "%n %u",
+            :units => {
+              :deci => {:one => "Tenth", :other => "Tenths"},
+              :unit =>  "u",
+              :ten => {:one => "Ten", :other => "Tens"},
+              :thousand => "t",
+              :million => "m",
+              :billion =>"b",
+              :trillion =>"t" ,
+              :quadrillion =>"q"
+            }
+          }
+        },
+        :percentage => { :format => {:delimiter => '', :precision => 2, :strip_insignificant_zeros => true} },
+        :precision => { :format => {:delimiter => '', :significant => true} }
+      },
+      :custom_units_for_number_to_human => {:mili => "mm", :centi => "cm", :deci => "dm", :unit => "m", :ten => "dam", :hundred => "hm", :thousand => "km"}
+    end
+
+    def test_number_to_i18n_currency
+      assert_equal("&$ - 10.00", number_to_currency(10, :locale => 'ts'))
+      assert_equal("(&$ - 10.00)", number_to_currency(-10, :locale => 'ts'))
+      assert_equal("-10.00 - &$", number_to_currency(-10, :locale => 'ts', :format => "%n - %u"))
+    end
+
+    def test_number_to_currency_with_empty_i18n_store
+      I18n.backend.store_translations 'empty', {}
+
+      assert_equal("$10.00", number_to_currency(10, :locale => 'empty'))
+      assert_equal("-$10.00", number_to_currency(-10, :locale => 'empty'))
+    end
+
+    def test_number_to_currency_without_currency_negative_format
+      I18n.backend.store_translations 'no_negative_format', :number => {
+        :currency => { :format => { :unit => '@', :format => '%n %u' } }
+      }
+
+      assert_equal("-10.00 @", number_to_currency(-10, :locale => 'no_negative_format'))
+    end
+
+    def test_number_with_i18n_precision
+      #Delimiter was set to ""
+      assert_equal("10000", number_to_rounded(10000, :locale => 'ts'))
+
+      #Precision inherited and significant was set
+      assert_equal("1.00", number_to_rounded(1.0, :locale => 'ts'))
+    end
+
+    def test_number_with_i18n_delimiter
+      #Delimiter "," and separator "."
+      assert_equal("1,000,000.234", number_to_delimited(1000000.234, :locale => 'ts'))
+    end
+
+    def test_number_to_i18n_percentage
+      # to see if strip_insignificant_zeros is true
+      assert_equal("1%", number_to_percentage(1, :locale => 'ts'))
+      # precision is 2, significant should be inherited
+      assert_equal("1.24%", number_to_percentage(1.2434, :locale => 'ts'))
+      # no delimiter
+      assert_equal("12434%", number_to_percentage(12434, :locale => 'ts'))
+    end
+
+    def test_number_to_i18n_human_size
+      #b for bytes and k for kbytes
+      assert_equal("2 k", number_to_human_size(2048, :locale => 'ts'))
+      assert_equal("42 b", number_to_human_size(42, :locale => 'ts'))
+    end
+
+    def test_number_to_human_with_default_translation_scope
+      #Using t for thousand
+      assert_equal "2 t", number_to_human(2000, :locale => 'ts')
+      #Significant was set to true with precision 2, using b for billion
+      assert_equal "1.2 b", number_to_human(1234567890, :locale => 'ts')
+      #Using pluralization (Ten/Tens and Tenth/Tenths)
+      assert_equal "1 Tenth", number_to_human(0.1, :locale => 'ts')
+      assert_equal "1.3 Tenth", number_to_human(0.134, :locale => 'ts')
+      assert_equal "2 Tenths", number_to_human(0.2, :locale => 'ts')
+      assert_equal "1 Ten", number_to_human(10, :locale => 'ts')
+      assert_equal "1.2 Ten", number_to_human(12, :locale => 'ts')
+      assert_equal "2 Tens", number_to_human(20, :locale => 'ts')
+    end
+
+    def test_number_to_human_with_custom_translation_scope
+      #Significant was set to true with precision 2, with custom translated units
+      assert_equal "4.3 cm", number_to_human(0.0432, :locale => 'ts', :units => :custom_units_for_number_to_human)
+    end
+  end
+end