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

[Bug]: Error: [Dinero.js] Amount is invalid. #734

Open
2 tasks done
joelmsanto opened this issue Apr 16, 2023 · 3 comments
Open
2 tasks done

[Bug]: Error: [Dinero.js] Amount is invalid. #734

joelmsanto opened this issue Apr 16, 2023 · 3 comments
Assignees
Labels
bug Something isn't working

Comments

@joelmsanto
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Current behavior

The following implementation is triggering an error. Is it possible to multiply by a decimal number?

const distanceRateInCents = 56;
const distanceInMiles = 2.0659309759999998;

multiply(dinero({amount: distanceRateInCents, currency: USD}), distanceInMiles)

Expected behavior

it should not throw an error

Steps to reproduce

node environment

Version

2.0.0-alpha

Environment

node js

Code of Conduct

  • I agree to follow this project's Code of Conduct
@joelmsanto joelmsanto added the bug Something isn't working label Apr 16, 2023
@pnappa
Copy link

pnappa commented Apr 20, 2023

As per the multiply docs (https://v2.dinerojs.com/docs/api/mutations/multiply)

If you need to multiply by a fractional multiplier, you shouldn't use floats, but scaled amounts instead. For example, instead of passing 2.1, you should pass { amount: 21, scale: 1 }. When using scaled amounts, the function converts the returned objects to the safest scale.

You cannot provide a floating point number as an argument to multiply, only integers. So, the literal "correct" way for your code snippet is:

multiply(
  dinero({ amount: distanceRateInCents, currency: USD }),
  { amount: 20659309759999998, scale: 16 }
)

And, the result of toDecimal on that number is: 1.156921346560000000.

But, per this example, what are you intending to measure? From the code, I am guessing you're attempting to price distance travelled, with each mile costing 56 cents. However, how do you want to round your numbers? Are you wishing for the result to be 56 cents per mile, rounded to the nearest cent? If so, rounding up, or rounding down? Do you really have a mile measurement accurate to 16 decimal places?

If I were in your shoes, I probably would write a function that performs reasonable rounding.

const calculateTripCost = (milesTravelled: number, centsPerMile: number): Dinero<number> => {
  // Simple check to see if a value is an integer
  if (Math.floor(centsPerMile) !== Math.ceil(centsPerMile)) {
    throw new Error('Please provide cents per mile as an integer');
  }
  const mileRate = dinero({ amount: centsPerMile, currency: USD });
  // Measure miles travelled to 6 decimal places, rounded.
  const milesInt = Math.round(milesTravelled * (1e6));
  const estCost = multiply(mileRate, { amount: milesInt, scale: 6 });
  // Normalise back to whole cents. This rounds using banker's rounding, but you can
  // supply a custom divide function to handle rounding.
  // See additional info on transformScale here:
  // https://v2.dinerojs.com/docs/api/conversions/transform-scale
  return transformScale(estCost, /*scale*/ 2, /*rounding mode*/ halfEven);
};

For the same input values, invoking toDecimal(calculateTripCost(distanceInMiles, distanceRateInCents)) returns 1.16. Probably what you want.

@svarlet
Copy link

svarlet commented Apr 28, 2023

@pnappa What if we need that many decimals? and what if the number of decimals varies all the time: 1.5, 2.7483939, 1, 2.000001, 3.8, etc. ?

@pnappa
Copy link

pnappa commented Apr 29, 2023

I can guarantee you do not need that many decimals for a floating point, simply due to the fact that there are lots of floating point numbers, which, in base 10 have a very large number of digits after the decimal point 😁 In JS, numbers are 64bit floating point numbers, which can have 1074 trailing digits, if you trust the maths here https://stackoverflow.com/a/39838768 . When you print a float in JS, it's often truncated for legibility, and for good reason.

The situation here is a little bit like this xkcd comic:

However @svarlet , the occurrences of 6 in my code snippet could be replaced by a larger number, but you don't want to make it too big, integers stop being able to be exactly represented around the 2^53 mark (see: Number.MAX_SAFE_INTEGER). the code works for numbers like 1.5 anyway - e.g. 1.5*1e6=1500000.

Note that at the end of that function, we round to the nearest cent anyway, so if you want to be more accurate there, increase the scale. Increasing from 2 to 5, increases the accuracy to thousands of a cent. But, you're going to need to truncate this value somewhere, banks/payment systems deal with cents. Dinero lets you handle currency exactly, but at some point, whether as inputs, or outputs, you're going to need to make decisions about accuracy somewhere (and document them!)

Edit: The Dinero docs has information on this https://v2.dinerojs.com/docs/faq/how-can-i-create-dinero-objects-from-floats

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants