openexchangerates / accounting.js

A lightweight JavaScript library for number, money and currency formatting - fully localisable, zero dependencies.
http://openexchangerates.github.io/accounting.js
MIT License
4.95k stars 528 forks source link

accounting.toFixed() still has binary/float rounding issues #99

Open f3ndot opened 9 years ago

f3ndot commented 9 years ago
accounting.toFixed(-0.0000615, 6) // "-0.000062"
accounting.toFixed(-0.0000315, 6) //"-0.000031"

The first number "rounds down" towards negative infinity, the second number "rounds up" towards infinity. The reason being is that -0.0000615, when multiplied the 10^6 power, it gets screwed by the floating binary issues:

-0.0000615 * Math.pow(10, 6) // -61.50000000000001
Math.round(-61.50000000000001) // 62

That 1.0 x 10^-14 number is sufficient for Math.round() to "round down" towards negative infinity because its no longer a tie-breaker.

f3ndot commented 9 years ago

Putting it another way:

accounting.toFixed(-61.5, 0) // "-61"
accounting.toFixed(-6.15, 1) // "-6.1"
accounting.toFixed(-0.615, 2) // "-0.61"
accounting.toFixed(-0.0615, 3) //"-0.061"
accounting.toFixed(-0.00615, 4) //"-0.0061"
accounting.toFixed(-0.000615, 5) // "-0.00061"
// Then, suddenly:
accounting.toFixed(-0.0000615, 6) // "-0.000062"
elekktrisch commented 8 years ago

This is the same as https://github.com/openexchangerates/accounting.js/issues/94 and https://github.com/openexchangerates/accounting.js/issues/111

gustavlrsn commented 7 years ago

Fixed in PR #164. The case that didn't work now works:

// as before:
accounting.toFixed(-61.5, 0) // "-61"
// and now:
accounting.toFixed(-0.0000615, 6) // "-0.000061"

However, I noticed that the formatNumber method sends the absolute value to toFixed so that negative numbers get rounded "up" as positive numbers. Which means that

accounting.formatNumber(-61.5, 0) // "-62"

This is the same as before and I assume it is the correct behavior as this is an accounting library. Negative numbers often mean costs, and if the cost is -61.5 and you want to round to zero decimals, I guess you'd rather have -62 than -61. But clearer test specifications on this would be nice.

wjcrowcroft commented 7 years ago

Fixed by #164, leaving open for further discussion on rounding negative numbers up/down..