Fix number_to_currency regression in handling "-0.0"

[flavorjones]

This fixes a regression introduced in #42581, and is related to prior work in #39350 and #37865.

Summary

In a follow-up comment on #42581 it was noted that number_to_currency(-0.0) returned "-$0.00" which was a change from the method's previous behavior which returned "$0.00".

That regression came about because of the logic in number_to_currency which attempted to detect a "parse failure" by checking for == 0.0 (see #39350 for context on gracefully dealing with "alternate input formats").

This PR simplifies the logic of the method, explicitly handling the "invalid string" case and improving performance for both the case where input is a Float and where it's an invalid string.

Benchmarks

I benchmarked a Float, a valid string, and an invalid string against main and this PR:

data = {
  "float" => -0.10,
  "valid" => "-0.10",
  "invalid" => "-0,10",
}

data.each do |name, value|
  Benchmark.ips do |x|
    x.config warmup: 0, time: 15

    x.report([title, name].join(" ")) do
      ActiveSupport::NumberHelper.number_to_currency(value)
    end

    x.save! "#{name}.out"
    x.compare!
  end
end

The results:

Calculating -------------------------------------
           old float     16.092k (±12.7%) i/s -    230.421k in  14.890424s
Calculating -------------------------------------
           new float     16.515k (±12.8%) i/s -    236.124k in  14.886041s

Comparison:
           new float:    16515.0 i/s
           old float:    16091.9 i/s - same-ish: difference falls within error

Calculating -------------------------------------
           old valid     16.353k (±13.0%) i/s -    234.174k in  14.892008s
Calculating -------------------------------------
           new valid     16.441k (±12.8%) i/s -    235.305k in  14.889485s

Comparison:
           new valid:    16441.4 i/s
           old valid:    16352.6 i/s - same-ish: difference falls within error

Calculating -------------------------------------
         old invalid     39.056k (±13.9%) i/s -    535.071k in  14.793478s
Calculating -------------------------------------
         new invalid     47.552k (±13.6%) i/s -    649.486k in  14.776688s

Comparison:
         new invalid:    47552.3 i/s
         old invalid:    39056.1 i/s - same-ish: difference falls within error

[p8]

Great refactoring!
If the number isn't a valid float ("123.0_badstring") should it always return 0.0 instead of the invalid value?
Otherwise this might be another regression?

[flavorjones]

@p8 Thanks for asking, the previous behavior is actually to just slap a currency symbol on the front of invalid strings (with some light handling of "-"), so that "alternate formats" like -1,23 get formatted as -$1,23:

ActiveSupport::NumberHelper.number_to_currency("-123.0_badstring")
# => "-$123.0_badstring"

This behavior is preserved in the refactoring! And the test coverage is pretty good for this helper.

[flavorjones]

Maybe more directly to your point: the behavior of NumberToRoundedConverter is to check (again) if the number is valid and if not to just return the input string. Avoiding that expensive no-op is part of why invalid strings are so much faster with this patch.

[p8]

@flavorjones Thanks for the clarification! That makes sense. 😄