Implement power function (clean up) #76
Conversation
src/long.js
Outdated
| LongPrototype.power = function power(exp) { | ||
| var a = this; | ||
| if (a.eq(Long.ONE)) return a; | ||
| if (isLong(exp)) exp = exp.toInt(); |
There was a problem hiding this comment.
It's certainly questionable to use such a large exponent, of course, but I wonder if this is safe in that a value > 0x7fffffff respectively > 0xffffffff still yields a somewhat expected result?
There was a problem hiding this comment.
Would the preference be to keep exponent as a Long and manipulate it as a 64 bit value? I figured it would be faster to use 32 bit math, as exponent values larger than 64 would overflow on bases beyond 0 and 1 anyway. So at the time the thought was speed rather than safety. I could certainly put the 64 bit exp math back into it if that is desired. Just let me know.
There was a problem hiding this comment.
Or is the concern that it could quietly give bad values in the overflow case? Does it need some kind of range check or overflow error of some kind? One could throw if base > 1 and exp > 64 but that would only be a broad check and there are plenty of combos that would still overflow. Not sure if there is a way to detect overflows during the multiplication?
EDIT:
Though if there are only up to 64 possible integer exponents, one could have a look up table of the 64 max possible bases and compare those to the given base to potentially predict an overflow? (Well, 63 entries as one is special case... Actually 62, 2^64 would be overflow as max is one less than count.)
|
It is probably best not to let that operation silently overflow without throwing some kind of meaningful error. I think I've figured out how to check for overflow in a relatively efficient manner. If there is a cleaner way to go about it, I am open to suggestions. |
|
Looking back at this, I realized I could catch floating point exponents easily enough in the overflow check, so I tweaked it and added the necessary test. |
This is basically a clean up of the previous PR: #62.
Adds power function. See original PR for previous details and discussion. This also fixes a couple places where multiply could return a signed zero when the original was unsigned (which was mentioned in #72).