Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[bugfix] fix #5596: floating number calculation converter #5597

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 4 additions & 3 deletions src/lib/duration/constructor.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { normalizeObjectUnits } from '../units/aliases';
import { getLocale } from '../locale/locales';
import isDurationValid from './valid.js';
import multiply from '../utils/float-calculate';

export function Duration(duration) {
var normalizedInput = normalizeObjectUnits(duration),
Expand All @@ -19,9 +20,9 @@ export function Duration(duration) {
// representation for dateAddRemove
this._milliseconds =
+milliseconds +
seconds * 1e3 + // 1000
minutes * 6e4 + // 1000 * 60
hours * 1000 * 60 * 60; //using 1000 * 60 * 60 instead of 36e5 to avoid floating point rounding errors https://github.com/moment/moment/issues/2978
multiply(seconds, 1e3) + // 1000
multiply(minutes, 6e4) + // 1000 * 60
multiply(hours, 36e5); //using 1000 * 60 * 60 instead of 36e5 to avoid floating point rounding errors https://github.com/moment/moment/issues/2978
// Because of dateAddRemove treats 24 hours as different from a
// day when working around DST, we need to store them separately
this._days = +days + weeks * 7;
Expand Down
33 changes: 33 additions & 0 deletions src/lib/utils/float-calculate.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
export default function multiply(a, b) {
var bi1 = toBigInt(parseFloat(a)),
bi2 = toBigInt(parseFloat(b)),
mag1 = bi1.magnification,
mag2 = bi2.magnification,
num1 = bi1.num,
num2 = bi2.num;
return (num1 * num2) / (mag1 * mag2);
}

function toBigInt(floatNum) {
var bigInt = {
num: 0,
magnification: 1,
},
strNum,
len,
mag,
intNum;

if (Math.floor(floatNum) === floatNum) {
bigInt.num = floatNum;
return bigInt;
}

strNum = floatNum.toString();
len = strNum.length - strNum.indexOf('.') - 1;
mag = Math.pow(10, len);
intNum = Number(floatNum.toString().replace('.', ''));
bigInt.num = intNum;
bigInt.magnification = mag;
return bigInt;
}
9 changes: 9 additions & 0 deletions src/test/moment/duration.js
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,15 @@ test('instantiation from another duration', function (assert) {
});

test('explicit cloning', function (assert) {
var duration = moment.duration(1.1234, 'hours');
assert.equal(
duration.milliseconds(),
240,
'Float number calculation will not lose accuracy'
);
});

test('float number calculation', function (assert) {
var durationA = moment.duration(5, 'milliseconds'),
durationB = durationA.clone();
durationA.add(5, 'milliseconds');
Expand Down