6 ms·
This library has some huge hazards. let added = Dinero({ amount: 0.1 }).add(Dinero({ amount: 0.2 })) let result = added.equalsTo(Dinero({ amount: 0.3 }
by recursive 9y ago
This library has some huge hazards.
let added = Dinero({ amount: 0.1 }).add(Dinero({ amount: 0.2 }))
let result = added.equalsTo(Dinero({ amount: 0.3 }))
`result` is false! I can't believe there's a currency library that doesn't do correct decimal arithmetic.
https://codepen.io/recursive/pen/VXogvN?editors=0010 https://codepen.io/recursive/pen/VXogvN?editors=0010
- Jaxan 9y agoOn the other hand, it has 100% coverage and zero vulnerabilities! We really should teach people how to test properly and not just use metrics to sell a library :-(.
- deleted 9y ago[deleted]
- meritt 9y agoI think the inability to write effective tests is single biggest contributor to technical debt over the past decade. Ineffective tests are arguably more dangerous than no tests at all, due to what you pointed out: the "100% Coverage" banner is about as effective as those Verisign website seals that were so popular in the early 2000s.
- arkh 9y agoUnit test coverage calcify your code. People will resist change because "it breaks unit tests". The best are acceptance tests. But here the use of SaaS systems with no sandbox or locally deployable mocks mean this is hard to automate.
- Arnavion 9y agoI assume you're supposed to use a smaller denomination of your choice to avoid decimals. For example, the submitted page tells you to use cents for EUR values and there's presumably nothing that prevents you from using tenths or hundreths of cents as the base denomination if you want. Perhaps it should not allow you to create an instance with a non-integral `amount` ?
- deleted 9y ago[deleted]
- zestyping 9y agoI am guessing the author would say that you are supposed to only use integer amounts. Amounts are measured in cents, so $15 is `Dinero({amount: 1500, currency: 'USD'})`. It is a serious problem that `Dinero({amount: 0.1})` doesn't immediately throw an exception, though—if you design a library that horribly breaks on non-integer inputs, you must reject integer inputs loudly and immediately.
- sarahdayan 9y agoGood call, will fix.
- Waterluvian 9y agoHow often are fractions of a cent calculated and managed? I always assumed that if you're doing some serious finance work you track into fractions of a penny.
- jessaustin 9y agoSure, but using floats is still the wrong way to handle this. Much better to just decide how much accuracy you need and set up your fixed-point system to handle that. Additionally you might want e.g. rationals to handle any rate that multiplies amounts.
- PeterisP 9y agoIIRC you generally do not; accounting standards tend to expect that every "booking" is rounded, so only some intermediate results can ever be in fractions of a penny. This sometimes results in "adjustment" bookings to ensure that a bunch of deals match the required total, because the rounding introduces a difference - e.g. if you have interest accrual by day and the monthly total interest would be $xx.35, then you might accrue $yy.01 every day and $yy.05 on the last day - but not $yy.01166 every day. Prices are a different issue, you're likely to have prices that are fractions of a penny; but any inventory valuations (either as stock or sale) would again be rounded after multiplying the price with the item quantity. Perhaps this could use some integration with a general concept of unit dimensions, distinguishing values that are measured in dollars(cents) and values such as prices that are "dollars/item" and can become "dollars" only when multiplied by number of items.
- sarahdayan 9y agoThe amounts must be specified in cents, as floats can’t be relied upon to perform calculations. That was my #1 reason to design the library using solely integers. I wrote all about it here: https://frontstuff.io/how-to-handle-monetary-values-in-javascript https://frontstuff.io/how-to-handle-monetary-values-in-javas...
- xab9 9y agoI've read the post and I do get the point BUT we currently support 25 languages, use accounting.js (I know, it's just formatting) and the backend returns float values and we still get by. I'm aware of the risks and not happy about it, but probably good will and magic holds it together (plus backend has a veto over the price with the final, canonical value on their side). Would it be possible to rewrite this library to use "floatish" numbers with initial multiplication and flooring then using decimal.js? I understand that non-internal arithmetics are slow as hell, but most of the time people just add products to a cart or sometimes apply a percentage discount. On the other hand, please do throw for non-integer values :)
- davvolun 9y agoIt's not (solely) an issue of speed. http://weitz.de/ieee/ http://weitz.de/ieee/ Plug in 0.11 and 0.1 and click `+`. You expect 0.21, but since 1/10 isn't cleanly represented in base 2 using a finite number of bits... (just like 1/3 isn't cleanly represented in base 10 ~0.33..). And when it comes to currency, rounding and "good enough" is probably not going to be good enough.
- xab9 9y agoIt may be totally silly (I consider myself lucky for getting by with good enough so far:)), but would `(Math.floor(0.11x10xx2)+Math.floor(0.1x10xx2))` divided by 100 work? Probably I'd like to do the last step with a custom arithmetic and not store the final value as a float, but like I said, I'm not sure. I get that the lowest denominator route is good, but it feels complicated and the chance of migrating a shoddy "good enough" project is zero in that case. (For fuck's sake, in 2018 formatting text or using utf8 on hacker news is too much to ask for, it's even worse than slashdot)
- nodesocket 9y agoLike the Stripe model, amounts should be in cents (integers), don't use decimals. Functions such as allocate() are super useful: // returns an array of two Dinero objects // the first one with an amount of 502 // the second one with an amount of 501 Dinero({ amount: 1003 }).allocate([50, 50])
- deleted 9y ago[deleted]