MikeMcl / bignumber.js

A JavaScript library for arbitrary-precision decimal and non-decimal arithmetic
http://mikemcl.github.io/bignumber.js
MIT License
6.69k stars 742 forks source link

Function isPositive returns true for 0 #194

Closed barakman closed 6 years ago

barakman commented 6 years ago

IMO, this is either a bug, or a very misleading function-naming.

Here is a simple way to reproduce:

let x = new BigNumber(0);
console.log(x.isPositive());

According to the documentation of the function, it is indeed a bug:

Returns true if the value of this BigNumber is positive, otherwise returns false.

That is, unless you change the word value to sign. But even then, the function name will remain extremely misleading (I just spent a very long time debugging my code until realizing that this seemingly-simple function was returning the opposite of what I was expecting it to).

The same problem may occur with function isNegative returning true for a zero value.

I suggest to keep the documentation as is, and update the implementation of these two functions to verify that the number is not zero.

For example:

    P.isPositive = function () {
      return this.s > 0 && !this.isZero();
    };

Thanks

MikeMcl commented 6 years ago

There is a negative and positive zero here because JavaScript's Number type has them:

Note that there is both a positive zero and a negative zero.

And negative zero can be useful to indicate that a value less than zero has been rounded to zero.

I agree that it would be better to use the word "sign" rather than "value", but I don't agree that the use of the latter makes it a bug - the value zero here is either positve or negative.

Thanks for the feedback, I will consider changing the wording in the next release.

MikeMcl commented 6 years ago

Changed the word "value" to "sign" in v8.0.0.

Malabarba commented 2 years ago

Hi there. First of all, thanks for all the effort that went into this package.

I understand that you can have a bignum 0 with a positive or negative sign. Still, I just wanted to pitch in here to say that (new BigNumber(0)).isPositive() returning true was a very unexpected behavior for me. Although it is working as documented, anywhere I look I would expect x.isPositive() to behave like x > 0.

Not saying you should change how the function works (it's probably too late for that), do what you feel is best.