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 532 forks source link

simplify formatNumber logic #3

Open millermedeiros opened 13 years ago

millermedeiros commented 13 years ago

no need to check if number is negative or do multiple substr.. A single RegExp is enough, check my implementation: http://stackoverflow.com/questions/149055/how-can-i-format-numbers-as-money-in-javascript/3284302#3284302

wjcrowcroft commented 13 years ago

Thanks, will take a look at it and reply in a bit!

wjcrowcroft commented 13 years ago

I've taken a look at the stackoverflow answer and played around with the function and regex you suggest - I'm getting this issue for large numbers though, which maybe you can help out with:


numberToCurrency(1234567890.99)  // "1,234,567,890.99"
numberToCurrency(123456789012345678.99)  // "123,456,789,012,345,680.00"
numberToCurrency(12345678901234567890.99)  // "12,345,678,901,234,567,168.00"

Looks like it's losing some info off the end of the number, after a certain number of digits. Any ideas why?

Otherwise, it seems good and slightly better optimised, so if we can make sure there are no hidden issues with it, I'm happy to try incorporating it.

millermedeiros commented 13 years ago

This happens because of 64-bit floating point Numbers. The maximum integer you can have without loosing precision is Math.pow(2, 53) == 9007199254740992 even tho Number.MAX_VALUE is way higher than that (same happens for negative numbers).

This bug can be replicated by:

console.log(Math.pow(2, 53) == (Math.pow(2, 53) + 1)); // true

Another thing that is good to note is that some operators only accepts integers until Math.pow(2, 31) - 1 == 2147483647 (positive or negative - if they deal with Int values - like bitwise operations)... if operator deals only with unsigned ints it works from 0 till Math.pow(2, 32).

wjcrowcroft commented 13 years ago

Great explanation, thanks. If that's the only issue and it's language-level (similar to how (0.615).toFixed(2) === "0.61") I don't see an issue with trying to implement this regex into the number formatting - I'll have a play around tomorrow and see what works.

Cheers!

wjcrowcroft commented 13 years ago

FYI - there are a few issues with the function you posted on StackOverflow:

precision of 0 is always forced to 2 by nDecimalDigits = nDecimalDigits || 2;, so that:

numberToCurrency(12345, '.', ',', 0) // "12,345.00"

If I replace that nDecimalDigits with nDecimalDigits = !nDecimalDigits ? "." : nDecimalDigits; (check for undefined as opposed to 0), the regex fails, so no thousand separators are inserted:

numberToCurrency(12345, '.', ',', 0) // "12345"
numberToCurrency(12345, '.', ',', 1) // "12,345.0"

I checked out the regular expression and looks like it can't handle numbers without decimal places. I'm not amazing at regexes though, so if you could suggest a fix for that, I'll be happy to include it in the library with attribution :)

Any ideas? thanks!