Skip to content

Commit

Permalink
fix(numberFilter): format large numbers correctly
Browse files Browse the repository at this point in the history
format large number to correct form instead of NAN.00 currently.
in previous code when first time we add the exponent with fractionSize
(number.toString()+'e'+fractionsize)
 and then convert it to an integer using ,
+(number.toString()+'e'+fractionsize)
when the number is very large and number will be represented by using exponent. for example 12345868059685210000 will be represented by 1.234586805968521e+21. as getting string in exponent is not handles, we get the number as NAN.
Handle when the number is being represented in exponent form, by adjusting exponent of number with fraction size:

Closes angular#8674
  • Loading branch information
gauravaror committed Aug 21, 2014
1 parent 8863b9d commit 57c6c0e
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 1 deletion.
8 changes: 7 additions & 1 deletion src/ng/filter/filters.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,12 @@ function numberFilter($locale) {
};
}

function getExponentString(numberString,fractionSize) {
var numberArray = numberString.split('e'),
fractionUsed = +fractionSize;
return (numberArray[0] + 'e' + (numberArray[1] ? (+numberArray[1] + fractionUsed) : fractionUsed));
}

var DECIMAL_SEP = '.';
function formatNumber(number, pattern, groupSep, decimalSep, fractionSize) {
if (number == null || !isFinite(number) || isObject(number)) return '';
Expand Down Expand Up @@ -151,7 +157,7 @@ function formatNumber(number, pattern, groupSep, decimalSep, fractionSize) {
// safely round numbers in JS without hitting imprecisions of floating-point arithmetics
// inspired by:
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Math/round
number = +(Math.round(+(number.toString() + 'e' + fractionSize)).toString() + 'e' + -fractionSize);
number = +(getExponentString(Math.round(+(getExponentString(number.toString(),fractionSize))).toString(),-fractionSize));

This comment has been minimized.

Copy link
@lgalfaso

lgalfaso Aug 21, 2014

if getExponentString does the casting from a string to a number, then this line would be a lot easier to read (and rename the function so the name represents what it does)

This comment has been minimized.

Copy link
@gauravaror

gauravaror Aug 22, 2014

Author Owner

I think its better to move both:
1 casting from number to string while send parameters.
2. conversion from string to number for returned value.

Will do the change.

Th function basically does both divides or multiples the number by 10(fraction size) passed by adding the fractionsize in exponent for:
How does the name sound: getMultipliedNumber since it does both multiplication of e(fractionSize) based on fractionSize in params positive and negetive it either division or multiplication.
i know this is not the most relevant name, can someone suggest one ?

This comment has been minimized.

Copy link
@lgalfaso

lgalfaso Aug 22, 2014

How does the name sound: getMultipliedNumber since it does both multiplication of e(fractionSize) based on fractionSize in params positive and negetive it either division or multiplication.

I like small descriptive names... I would just call it timesPow10

This comment has been minimized.

Copy link
@gauravaror

gauravaror Aug 22, 2014

Author Owner

i like the name, would change it to timesPow10
Thanks 👍


var fraction = ('' + number).split(DECIMAL_SEP);

This comment has been minimized.

Copy link
@jbedard

jbedard Aug 21, 2014

Is it possible for this line to also produce a #.###e# string and then the decimal is split wrong?

This comment has been minimized.

Copy link
@gauravaror

gauravaror Aug 22, 2014

Author Owner

I think that case will never arise, when the number enters the numberFilter we check for if number is in exponent form, if it is we handle it in different form.

Number is only represent in exponent form if it is greater then 1e21 . But since our expression is above is keeping the number same, so case of it being smalled than 1e21 and after the expression getting bigger than 1e21 will never arise. So we should be safe.

I think we should not handle this impossible case. until you think its need.

Thanks for the pointing out, it could have been a real bug 👍

var whole = fraction[0];
Expand Down
6 changes: 6 additions & 0 deletions test/ng/filter/filtersSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,12 @@ describe('filters', function() {
expect(num).toBe('1.1112');
});

it('should format large number',function() {
pattern.gsize = 2;
var num = formatNumber(12345868059685210000, pattern, ',', '.', 2);
expect(num).toBe('12,345,868,059,685,210,000.00');
});

it('should format according different separators', function() {
var num = formatNumber(1234567.1, pattern, '.', ',', 2);
expect(num).toBe('1.234.567,10');
Expand Down

0 comments on commit 57c6c0e

Please sign in to comment.