Open lededje opened 9 years ago
Yeah, I see the issue, the correctionFactor
is not working quite right. I am thinking it'd be easier to not use reduce and just get the correction from the accumulator and the current value instead. I only see 2 parameters passed to this function.
I have no idea, never looked at this part of the code (yet?)
I was thinking about a solution:
function correctionFactor() {
var args = Array.prototype.slice.call(arguments);
return args.reduce(function(prev, next) {
var mn = multiplier(next);
return prev > mn ? prev : mn;
}, 1);
}
Could work since this code is looking for the largest multiplier. I'll submit a PR sometime today or tomorrow (mostly for test).
Yeah, it's never that easy.
So far all I can come up with is complicated looking code involving log10
and toFixed
yep, and this all code part is not the awesomest :smile:
If
multiply: function(value) {
function cback(accum, curr) {
var corrFactor = correctionFactor(accum, curr),
fixedDigits = Math.log(corrFactor)/Math.LN10,
result = parseFloat((accum * corrFactor).toFixed(fixedDigits));
result *= parseFloat(curr * corrFactor).toFixed(fixedDigits));
result /= corrFactor * corrFactor;
return result;
}
this._value = [this._value, value].reduce(cback, 1);
return this;
}
was the fix, would it cause issues with other cases of the floating point error.
It would be nice to see where else the floating point issue would appear to see if this is the fix for it.
Some example cases
64.395 * 1000 === 64.395 // actual value is 64.394999999999
64.395 * 42.788 === 2755.33326 // actual value is 2755.3332599999994
the easy ones are
.28 * 100
=> 28.000000000000004
.29 * 100
=> 28.999999999999996
.56 * 100
=> 56.00000000000001
.57 * 100
=> 56.99999999999999
numeral(64.43).multiply(100);
// 6443.000000000001I know this is a floating point issue but I thought that is what this library was meant to sort out.