Using floats to represent money

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Using floats to represent money

Greg Navis
Hi!

Some time ago, I opened the issue 27645 where I suggested we improve the code and docs regards representing money. Currently, it seems that floats are encouraged (see the examples in the docs) which is a bad practice. Please see my comment on the issue to understand why it leads to problems.

Someone experienced in working with financial systems knows to avoid this pitfall but newcomers are likely to build a buggy system. I'm happy to contribute patches to both the code and docs but first I wanted to ask:

How do we want Rails to handle this?

Best regards
Greg Navis

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Using floats to represent money

Xavier Noria
I wouldn't go as far as to say floats are encouraged in general, you gave an example of a questionable sample code, but also have counterexamples, for example in the type of price attributes in the AR migrations guide. Bottom line, all Rails core is aware of these gotchas, anything that could be misleading in this sense is probably an overlook.

Problem is this helper rarely gets a literal, and

    number_to_currency(1234567890.50)

reads much better than

    number_to_currency("1234567890.50".to_d)

for my taste.

It's easy to make something faster if you can make it wrong, and also to make it read better by stretching the truth just a little bit 😃.

Maybe we could add a sentence saying "The following examples use floats, however, numbers representing money normally need and exact type like BigDecimal". Also open to better suggestions.


--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Using floats to represent money

Hugo Peixoto
An alternative could be using

    number_to_currency(1234567890.50r)



On Sat, Jul 29, 2017 at 9:13 AM, Xavier Noria <[hidden email]> wrote:
I wouldn't go as far as to say floats are encouraged in general, you gave an example of a questionable sample code, but also have counterexamples, for example in the type of price attributes in the AR migrations guide. Bottom line, all Rails core is aware of these gotchas, anything that could be misleading in this sense is probably an overlook.

Problem is this helper rarely gets a literal, and

    number_to_currency(1234567890.50)

reads much better than

    number_to_currency("1234567890.50".to_d)

for my taste.

It's easy to make something faster if you can make it wrong, and also to make it read better by stretching the truth just a little bit 😃.

Maybe we could add a sentence saying "The following examples use floats, however, numbers representing money normally need and exact type like BigDecimal". Also open to better suggestions.


--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Using floats to represent money

Greg Navis
I would say that if a piece of code dealing with money uses floats then it's wrong. It's simply the wrong datatype for the job. It can be used for scientific and engineering computations where the amounts we're dealing with are always uncertain. This isn't the case for monetary calculations though.

I would say: if your code is dealing with money and uses floats then it's wrong.

I can see at least three representations for money two of which you've already mentioned (BigDecimal and Rational). Personally, I prefer to use integers and count cents. For example, I'd write $123.45 as 123_45.

Using Rational or BigDecimal can be problematic when dealing with external systems. For example, I saw a system that issued payment requests in sub-cent precision. The result was that the payment provider rounded these numbers and things didn't add up in the database because the actual payments differed from the requested ones.

To sum up, the problems I see are the following:

1. number_to_currency expects a type with fractional part. I can't just pass it 123_45 (= 12345) and expect it to output $123.45.
2. NumberToCurrencyConverter explicitly uses floats and rounding. I think they should be replaced with something equivalent to number.divmod(100) (where the precision, 100 in this example, could be configurable). I realize the implications for backward compatibility. I'm just talking about the ideal future code of this helper.
3. The docs for number_to_currency use floats. I think it's irrelevant that literals are seldom used. Storing these values in a database field doesn't change my point - floats aren't the right representation for money.

Best regards
Greg Navis

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Using floats to represent money

Xavier Noria
This is not a thread about best practices representing money. We all know in general[*] you want an exact type. I have personally used arbitrary precision when enough, rationals when I have had requirements for exact division, and also integers.

Rather, this is a thread about the examples in the documentation of #number_to_currency. Let's focus the discussion on this topic.

Generally speaking, examples in documentation should be realistic (ie, class User < AR::Base in Rails communicates better than class C < AR::Base), and should promote best practices and idiomatic code implictly. Thet have to be easily understandable too.

This helper is in a gray area, because you need to write literals to show how it behaves, and that is not realistic at all. Applications have their money quantities stored somewhere.

If you use a rational literal with a suffix:

    number_to_currency(1234567890.50r)                # => $1,234,567,890.50
    number_to_currency(1234567890.506r)               # => $1,234,567,890.51
    number_to_currency(1234567890.506r, precision: 3) # => $1,234,567,890.506
    number_to_currency(1234567890.506r, locale: :fr)  # => 1 234 567 890,51 €

the number of WTFs in readers is going to be greater than any positive integer. Why is it passing a rational literal? Do I need to pass a rational?

If we use #to_d

    number_to_currency("1234567890.50".to_d)                # => $1,234,567,890.50
    number_to_currency("1234567890.506".to_d)               # => $1,234,567,890.51
    number_to_currency("1234567890.506".to_d, precision: 3) # => $1,234,567,890.506
    number_to_currency("1234567890.506".to_d, locale: :fr)  # => 1 234 567 890,51 €

readers are going to be equally puzzled. That is obscuring the understanding of the example, which is to compare calls and comments, left and right, and see how the options alter the output. That is the point.

    number_to_currency(1234567890.50)                    # => $1,234,567,890.50
    number_to_currency(1234567890.506)                   # => $1,234,567,890.51
    number_to_currency(1234567890.506, precision: 3)     # => $1,234,567,890.506
    number_to_currency(1234567890.506, locale: :fr)      # => 1 234 567 890,51 €

That one has less noise for my taste, allows the reader to process the examples in a snap, and serves better to the purpose of this sample code. It does not convey a best practice, that is true, and that is where you have to choose a trade-off. Looking at the three options, I prefer compromising towards readability, the third one.

Then you compensate with a warning like the one I suggested before, and done.

Xavier

[*] Let me also say that there are use cases for floats. If you have a personal app to track your expenses, or whatever use case you have in which you just don't care about the possibility of a missing cent here or there, you are fine with floats.

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.
Loading...