kip

kip

ex_cldr Core Team

Money.new/2 does accept a float - and perhaps it should not. It accepts an integer, Decimal and String as well which are clearly preferable. It converts to a Decimal at the interface. The inspect output you reference above is just that. In #Money<:USD, 200.012356> the 200.012356 is not a float, its a Decimal.

So the only place a float gets in the picture is at the factory interface new/2. There is no internally usage of a float whatsoever. Even to the extent I had to build my Decimal math lib to support basic finance calculations.

Definitely open to community views on disallowing floats even at the factory interface.

A coda to the above: One reason that floats are supported in Money.new/2 is that services, like OpenExchangeRates, return data in json format which the Elixir decoders will understandably reduce to a float when given the following:

    rates: {
        AED: 3.672538,
        AFN: 66.809999,
        ALL: 125.716501,
        AMD: 484.902502,
        ANG: 1.788575,
        AOA: 135.295998,
        ARS: 9.750101,
        AUD: 1.390866,
        /* ... */
    }

So removing float support at in new/2 would create some challenges with the exisiting services that deliver floats. Not insurmountable, but not helpful either.

0
Post #31
OvermindDL1

OvermindDL1

Absolutely disallow. Consuming or returning floats should be a hard error, absolutely not allowed as it will eventually break.

Such services should be disallowed. Anyone who uses a float to transfer monetary values is not just recklessness, but also malicious. Storing a number as a string allows lossless conversion and that is fine, same with fixed point, integers, ratio’s, etc, but storing it as a float should always be absolutely disallowed.

2
Post #32
kip

kip

ex_cldr Core Team

We would all easily agree that touching floating point arithmetic would be a very bad thing. But as best I can tell, simply converting from a float to a Decimal would not appear to introduce any additional precision errors.

That assumption is based upon my reading of IEE 754, consulting the Erlang and Decimal docs and running millions of empirical tests in the last hour or so round tripping Float → Decimal → Float (using :random.uniform/0 so not conclusive as yet). I also reviewed how Poison parses numbers and it ultimately uses String.to_float/1 which calls :erlang.binary_to_float/1 which is the same code path I used in my testing.

But I’m not a mathematician therefore I’m not claiming I have a definitive understanding whatsoever. However I definitely want to get to a clear and specific understanding here so I can make the best informed decisions on what I need to do in the lib given the goals I have expressed for it in the readme.

I’d be very happy to be directed to the relevant literature on the simple “conversion” path since all content I can find in the last hour have been about arithmetic about which we have no dispute.

Of course this doesn’t change your second challenge around some very popular external services that deliver exchange rates in json number format (and therefore reduced to a Float by json decoders).

2
Post #33
axelson

axelson

Scenic Core Team

I think the problem is not allowing the conversion per-se, but by allowing the conversion from floats then some application developers will not realize that they are dealing with an imprecise format and the potential for hard-to-detect errors. So in a way the library could be seen as encouraging bad practices. As an alternative new/2 could raise an error describing the issue when called with a float.

2
Post #34
kip

kip

ex_cldr Core Team

I think the problem is not allowing the conversion per-se, but by allowing the conversion from floats then some application developers will not realize that they are dealing with an imprecise format

Its good point of course.

And after writing a much longer answer with questions I realise all my issues are not about Money.new/2 but are about doing arithmetic (interest rate calcs, exchange rate calcs) which don’t involve Money.new/2, just conversion of float to Decimal in order to become a multiplicand.

I’ll deprecate Money.new(currency, Float) in the next version and remove it in ex_money 2.0.

Great conversation, thanks to you all.

3
Post #35
OvermindDL1

OvermindDL1

Yes, yes it would. :wink:

Say you get a number from json or so, not a string, a number, and it is like 8.10, well 8.10 is not perfectly representable so you’d probably get a 8.0999999999999999* decimal, and this is just one of the most simple examples.

Never even accept float!

For note, 8.099999999999999 will (with enough 9’s) be rendered to the screen or so as 8.1, but will still internally have lots of 9’s that will mess up calculations. You cannot perfectly represent Base 10 floating point in Base 2 floating point, it is impossible for many numbers.

3
Post #36
sorentwo

sorentwo

Oban Core Team

While it isn’t on the topic of floats and their evil use within the realm of money, I’d like to mention that I’m the maintainer of the Braintree library for Elixir.

I’ve personally never changed payment gateways during a project and don’t see a pressing need for an all encompassing payment gateway, but I know other people coming to the language will be looking for it. Best of luck!

Please reach out or feel free to lift the XML handling from the Braintree library.

4
Post #37
ashish173

ashish173

CURRENT STATUS: we have merged the implementation in the dev branch; we’ll be releasing a new version in a day or two.

PR# MERGED: Introducing the Money protocol by oyeb · Pull Request #71 · aviabird/gringotts · GitHub
ISSUE# Replace floating point numbers with a Money lib · Issue #62 · aviabird/gringotts · GitHub

1
Post #38
kip

kip

ex_cldr Core Team

I’ve released ex_money version 2.0.0 which no longer supports a float as a parameter to the factory method Money.new/2 (it will return an error or raise on the ! method).

I have also added Money.from_float/2 which will accept a float parameter. The intention is to make it really clear to a developer that they are using something out of the ordinary. As a precaution to ensure that ex_money doesn’t introduce any additional imprecision it will return an error if the supplied float has more than 15 digits of precision.

(not really Gringotts related I know but I want to reflect the decisions I took based upon the solid feedback from the community)

8
Post #39
josevalim

josevalim

Creator of Elixir

Hi @pkrawat1 and team, I would like to reinforce the point above.

Using a GenServer when you don’t need one is an anti-pattern and it will actually have severe applications on the system performance, as you put all operations besides a single process which will become a bottleneck. We cover this in detail in Adopting Elixir and there is a thread about this particular topic in the forum.

Even if you may end-up using the worker for configuration, I would ask you to revisit that, because it should not be the place of libraries to impose a stateful configuration mechanism. Simply ask the users to pass the configuration every time the gateway is called. If the users of your library need to store this configuration somewhere, they can easily do so by using an Agent, application config, GenServer, etc.

11
Post #40