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

Using comparison operators with Decimals can give the wrong result #172

Open
kynan opened this issue Mar 31, 2021 · 4 comments · May be fixed by #186
Open

Using comparison operators with Decimals can give the wrong result #172

kynan opened this issue Mar 31, 2021 · 4 comments · May be fixed by #186

Comments

@kynan
Copy link

kynan commented Mar 31, 2021

The following behavior is somewhat confusing:

const { Decimal } = require('decimal.js');
five = new Decimal(5);
ten = new Decimal(10);
five < ten   // false
five <= ten  // false
ten > five   // false
ten >= five  // false

I'm aware there's dedicated comparison operators, but why does "elementary" comparison give the wrong result in this case?

@MikeMcl
Copy link
Owner

MikeMcl commented Mar 31, 2021

The relational operators cause valueOf to be called on each Decimal, which returns string values which are then compared 'alphabetically', and '5' comes after '1'.

@kynan
Copy link
Author

kynan commented Apr 2, 2021

That does indeed make a lot of sense, thanks for the explanation! The joys of lexicographic comparison. And of course you can't have valueOf return a numeric value as that may lose precision.

Worth mentioning this in the docs as a caveat?

@MikeMcl
Copy link
Owner

MikeMcl commented Apr 2, 2021

As I did recently with big.js, in the next major release I may throw on calls to valueOf to prevent accidental usage of Decimals with arithmetic and relational operators.

@kynan
Copy link
Author

kynan commented Apr 3, 2021

That's even better, as people don't read documentation :)

@MikeMcl MikeMcl changed the title Elementary comparison operators give the wrong result Using comparison operators with Decimals can give the wrong result Jun 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants