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
feat: support toBigInt
method
#356
base: master
Are you sure you want to change the base?
Conversation
Nice work but more test cases are required. In the documentation you add the following example, y = new BigNumber('45987349857634085409857349856430985')
y.toBigInt() // 45987349857634085409857349856430985n but your implementation of A correct implementation would be the following. Note that I use P.toBigInt = function () {
if (!this.isInteger()) throw Error(bignumberError + 'Not an integer: ' + valueOf(this));
return BigInt(this.toFixed());
}; or, if we want to allow BigInts to be formed from non-integers, and to return P.toBigInt = function () {
return this.isFinite() ? BigInt(this.integerValue(1).toFixed()) : null;
}; where the The most efficient implementation would be the following. Like the P.toBigInt = function () {
if (!this.isInteger()) throw Error(bignumberError + 'Not an integer: ' + valueOf(this));
var str = coeffToString(this.c);
for (var i = this.e + 1 - str.length; i > 0; i--) str += '0';
return BigInt(this.s < 0 ? '-' + str : str);
}; Okay, so this PR needs your implementation of |
Thanks for taking the time here. This is a great implementation. I will replace it later |
I replaced and added some tests. Do we need more test cases? what tests need to add? |
No, it's all good. I'm still not 100% on including this because up to now this library uses ECMAScript 3 (ES3) features only and I am not sure I want to break that convention. I'll leave this PR open for now. Thanks for your efforts. |
We could determine if the current environment supports BigInt or else throw an error so that we can support the modern browser without affecting |
close: #352