scurker / currency.js

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

Simple divides and multiply generate an incorrect decimal part #468

Closed sndywlkns closed 6 months ago

sndywlkns commented 6 months ago

I'm running the following code, and the comments show the output for reach line. I believe the final 0.40 is incorrect, but wanted to ask if I'm doing this correctly.

This starts with a monthly salary of 2_164_000, and calculates the daily salary by dividing by 30, then it gets the hourly salary by dividing that by 8. If a person worked 15 days, the total worked hours is 8 * 15 = 120, so the last step calculates the salary the user should get after working 120 hours, which is half of the original salary.

So the expectation is that the final result should be 2164000 / 2 = 1082000, but it is 1082000.4

const salary = currency(2164000);
// salary
// intValue: 216400000
// p: 100
// s: {symbol: '$', separator: ',', decimal: '.', errorOnInvalid: false, precision: 2, …}
// value: 2164000

const dailySalary = salary.divide(30.0);
// dailySalary
// intValue: 7213333
// p: 100
// s: {symbol: '$', separator: ',', decimal: '.', errorOnInvalid: false, precision: 2, …}
// value: 72133.33

const hourlySalary = dailySalary.divide(8.0);
// hourlySalary
// intValue: 901667
// p: 100
// s: {symbol: '$', separator: ',', decimal: '.', errorOnInvalid: false, precision: 2, …}
// value: 9016.67

const totalSalaryFor120Hours = hourlySalary.multiply(120.0);
// totalSalaryFor120Hours
// intValue: 108200040
// p: 100
// s: {symbol: '$', separator: ',', decimal: '.', errorOnInvalid: false, precision: 2, …}
// value: 1082000.4

From what I can see, this is being caused by the rounding to two decimals of precision when hourlySalary is created. I could use { precision: 4 } when creating the first currency object, however:

  1. I thought the point of using this lib was to not have to do this kind of thing manually.
  2. The new result (1082000.004) is still incorrect, just that the error is smaller.

Am I missing something?

sndywlkns commented 6 months ago

Just for the sake of comparison, I rewrote this small test using decimal.js and it is returning the expected result:

const salaryD = new Decimal(2164000);
const dailySalaryD = salaryD.dividedBy(30.0);
const hourlySalaryD = dailySalaryD.dividedBy(8.0);
const totalSalaryFor120HoursD = hourlySalaryD.times(120.0); // <-- is 1082000
scurker commented 6 months ago

This is the expected outcome. decimal.js and currency.js work differently in that currency is geared towards maintaining the specified currency precision where decimal maintains arbitrary precision.

The issue with chaining results like you are in regards to currency is that we don't know the underlying intention between operations so each operation results in an instance that maintains the specified precision. For a comparative example, I took another currency library, Dinero, and the results match the output of currency.js:

Dinero({ amount: 216400000, precision: 2 }).divide(30).divide(8).multiply(120).getAmount()
// 108200040

If you need to maintain a higher precision between operations, you need to specify that as part of the object while alternatively specifying your final output precision:

const valueWithPrecision = currency(2164000, { precision: 4 }).divide(30).divide(8).multiply(120)
console.log(currency(valueWithPrecision, { precision: 2 }).format())
// $1,082,000.00