Skip to content
This repository has been archived by the owner on Apr 13, 2022. It is now read-only.

numberFilter unnecessary string to number to string conversion => precision issue #16175

Open
1 of 3 tasks
msperisen opened this issue Aug 15, 2017 · 3 comments
Open
1 of 3 tasks

Comments

@msperisen
Copy link

I'm submitting a ...

  • bug report
  • feature request
  • other (Please do not submit support requests here (see above))

Current behavior:
'9999999999999999' | number:0 => 10,000,000,000,000,000
9999999999999999 | number:0 => 10,000,000,000,000,000

Expected / new behavior:
'9999999999999999' | number:0 => 9,999,999,999,999,999
9999999999999999 | number:0 => 10,000,000,000,000,000

Minimal reproduction of the problem with instructions:
see plnkr

AngularJS version:
all

Browser:
all
Anything else:
the problem stems from an - in my opinion - unnecessary conversion from string to number to string in the formatNumber function. basically just to remove the sign from the string with Math.abs().
doing something like the snipped below would allow to correctly format string number presentation - without limits of number precision (up to the limits defined by the rest of the code).
in some cases this is enough and one does not have to go for a big(int|number) implementation with corresponding localised formatters.

var numStr =  isString(number) ? (number.startsWith('-') || number.startsWith('+') ? number.substring(1): number) : Math.abs(number) + ''
@Narretz
Copy link
Contributor

Narretz commented Aug 16, 2017

Hi, thanks for the report. The number conversion continues to be a neverending source of joy ...

The best way to test your observation is to make the change in the code, add a test, and see if it makes any other tests fail, then open a PR.

@Narretz Narretz added this to the Purgatory milestone Aug 16, 2017
@gkalpak
Copy link
Member

gkalpak commented Aug 16, 2017

Converting a numeric string to a number and back to string is necessary to ensure that it is a valid number representation (e.g. no locale-specific separators etc). If we omit that step, we need extra logic to ensure that (and it might be non-trivial; but happy to be proven wrong).

Floating Point Arithmetic is what it is - embrace it, don't try to fight it - just my 2 cents 😃

@lgalfaso
Copy link
Contributor

Unfortunately, Math.abs(...) does more than what people expect. Math.abs('5e5') == 500000 and Math.abs('0x55') == 85. Fortunately, in modern browsers Math.abs('033') == 33 /* != 033 */ (and do not get me started with handling of extra spaces). Any change in this line should take all these things into consideration :(

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants