Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

number_to_currency returns incorrect results #27645

Closed
gregnavis opened this issue Jan 11, 2017 · 3 comments
Closed

number_to_currency returns incorrect results #27645

gregnavis opened this issue Jan 11, 2017 · 3 comments

Comments

@gregnavis
Copy link
Contributor

Summary

#number_to_currency uses Float#to_s which causes a loss of precision.

In general, the docs seem to encourage using floats for representing monetary amounts which is incorrect, dangerous and should be avoided. The correct approach is to use either integers or decimals.

I think that in the short-term we should fix the issue with lack of precision but in the long-term we should make the helper issue a warning (or even raise an exception) if someone passes a float.

I'm happy to provide a fix but I wanted to discuss the issue with the community first.

Steps to reproduce

  1. Fire up a Rails console.
  2. Type:
helper.number_to_currency(0.10000000000000000555, precision: 30)

Expected behavior

I expected to get $0.100000000000000005550000000000.

Actual behavior

I got "$0.100000000000000000000000000000".

Root Cause

The cause is a call to Float#to_s in NumberToCurrencyConverter:

def convert
  number = self.number.to_s.strip
  format = options[:format]

  if number.to_f.negative?
    format = options[:negative_format]
    number = absolute_value(number)
  end

  rounded_number = NumberToRoundedConverter.convert(number, options)
  format.gsub("%n".freeze, rounded_number).gsub("%u".freeze, options[:unit])
end

The problem is that Float#to_s looses precision. To give you an example:

# It's not possible to represent 0.1 accurately in binary.
n = 0.1

# Calling n.to_s returns "0.1" which seems to be correct but ...
puts n.to_s

# is incorrect as n is *not* equal to 0.1.
printf '%.20f', n # "0.10000000000000000555"
puts

# So it turns out that the code below prints "0.1"
puts 0.10000000000000000555.to_s

System configuration

$ rails --version
Rails 4.2.6
$ ruby --version
ruby 2.2.2p95 (2015-04-13 revision 50295) [x86_64-linux]
@coreyward
Copy link
Contributor

Is it reasonable to assume that, when dealing with currency, precision of 10+ digits is an important factor? See Significant Figures while pondering.

@rails-bot
Copy link

rails-bot bot commented May 5, 2017

This issue has been automatically marked as stale because it has not been commented on for at least three months.
The resources of the Rails team are limited, and so we are asking for your help.
If you can still reproduce this error on the 5-1-stable branch or on master, please reply with all of the information you have about it in order to keep the issue open.
Thank you for all your contributions.

@rails-bot rails-bot bot closed this as completed May 12, 2017
@gregnavis
Copy link
Contributor Author

Thanks for the comment and sorry for a late reply @coreyward.

I don't think that the concept of significant figures applies to monetary calculations. They're used to deal with random errors in measurements. In financial applications, all digits are significant. Using floats often results in incorrect behavior. I saw a financial system send unwarranted payment requests because payments were made in installments and there was $0.01 left unpaid due to rounding errors.

Let me stress that I'm really concerned about a bigger thing: it seems Rails encourages (or at least doesn't discourage) using floating-point numbers for monetary calculations. This means code like the aforementioned #number_to_currency and documentation use floats for monetary operations.

Let me give you two examples adapted form Effective Java by Joshua Bloch.

Example 1

Imagine a site where users can pay for items using credits they've accumulated. I have $1.03 and buy an item for $0.42. I get $0.61 in change. To increase sales, the system has a recommender system that suggests items to buy that are within my budget. There's another item for $0.62 which, theoretically, shouldn't be recommend to me due to insufficient funds in my account. However, programmers used floats to represent money and I will get a recommendation because my real balance is:

[1] pry(main)> 1.03 - 0.42
=> 0.6100000000000001

Ooops.

Example 2

I have $1 in my pocket and I'm offered a row of candy. The first one costs $0.10, the second one - $0.20, the third one - $0.30, and so on. I can clearly buy 4 pieces of candy: $0.10 + $0.20 + $0.30 + $0.40 = $1. Well, not necessarily if I keep track of things using floats:

funds = 1.0
prices = [0.1, 0.2, 0.3, 0.4]
count = 0

prices.each do |price|
  next if price > funds

  funds -= price
  count += 1
end

puts funds # => 0.39999999999999997
puts count # => 3

@rails-bot rails-bot bot removed the stale label May 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants