scurker / currency.js

A javascript library for handling currencies
https://currency.js.org
MIT License
3.15k stars 142 forks source link

Bug - `fromCents` miscalculations in subsequent calculations #425

Open jwarykowski opened 1 year ago

jwarykowski commented 1 year ago

Hi,

I noticed a weird problem when using a fromCents instance when performing further calculations down the line, please see the example below:

  const itemSubTotalFromCents = currency('800000', { fromCents: true, precision: 4 })
  const itemSubTotalWithoutFromCents = currency('80.00', { precision: 4 })

  const invoiceDiscount = currency('20.00', { precision: 4 })
  const invoiceSubTotal = currency('80.00', { precision: 4 })

  const resultWithFromCents = invoiceDiscount.multiply(itemSubTotalFromCents.divide(invoiceSubTotal))

  // returns { intValue: 20, value: 0.002, s: Object, p: 10000 }

  const resultWithoutFromCents = invoiceDiscount.multiply(itemSubTotalWithoutFromCents.divide(invoiceSubTotal))

  // returns { intValue: 200000, value: 20, s: Object, p: 10000 }

You can see that the first example which uses the fromCents looks incorrect, if you don't pass this option I get the expected result. If possible can you confirm whether this is a bug or not? I've only had a quick look at this but any insight would be greatly appreciated. I wouldn't expect the fromCents option to be carried forward in the returning instance.

In order to get round this problem I can just wrap with another instance of currency and this works:

const itemSubTotalFromCentsWithWorkAround = currency(currency('800000', { fromCents: true, precision: 4 }), { precision: 4 })
const resultWithWorkaround = invoiceDiscount.multiply(itemSubTotalFromCentsWithWorkAround.divide(invoiceSubTotal))

// returns { intValue: 200000, value: 20, s: Object, p: 10000 }

Note I use fromCents originally as the BE returns the value in centi cents

scurker commented 1 year ago

That might be a bug, or at least I would probably flag that behavior is incorrect.

I need to look into this further, but I would think at a high level both of your results should return the same value.

jwarykowski commented 1 year ago

Hey @scurker, thanks for your response. Agree, I'll try and take a look at this if I get a chance.

paro-paro commented 1 year ago

Hi guys!

I ran into the exact same issue as @jwarykowski and used the same workaround for "fixing" it.

I believe the problem lies within the divide function that, as stated above, carries forward the fromCents property in the returned instance when (I think) it should not.

Using the same values as the example above...

let a, b, c, d

const numerator = currency('80.00', { precision: 4 })
const numeratorFromCents = currency('800000', { fromCents: true, precision: 4 })
const denominator = currency('80.00', { precision: 4 })

a = numerator.divide(denominator) // returns instance of currency(1, { fromCents: false, precision: 4 })
b = numeratorFromCents.divide(denominator) // returns instance of currency(1, { fromCents: true, precision: 4 })

console.log(a.value) // 1
console.log(b.value) // 0.0001

c = currency(1, { fromCents: false, precision: 4 })
d = currency(1, { fromCents: true, precision: 4 })

console.log(c.value) // which effectively should be 1
console.log(d.value) // which effectively should be 0.0001

Maybe the returned instance of the divide function should "always" have the fromCents property set to false (which is what the workaround actually does)?

Or at least when is the result of dividing two currency objects?

Thanks in advanced!

cydside commented 7 months ago

Hi, here is a similar issue:


const EURO = value => currency(value, { fromCents: true, symbol: '€', decimal: ',', separator: '.' });

function myFunction() {
let tottax = EURO(3000);
let vatrate = EURO(2200);
let totvat = EURO(tottax.divide(100).multiply(vatrate.value).intValue);

console.log(`tottax.value: ${tottax.value}`);
console.log(`totvat.value: ${totvat.value}`);

let totamo = totvat.add(tottax.value);

console.log(`vatrate: ${vatrate.value}<br>tottax: ${tottax.value}<br>totvat: ${totvat.value}<br>totamo: ${totamo.value}`);
}

It's reporting totamo as 6.9 instead of 36.6 even if tottax and totvat are storing the correct values.