Fix BigInteger out of range bug, cause of using long value. #284
Conversation
|
@Sunshow Please sign the Contributor License Agreement! Click here to manually synchronize the status of this Pull Request. See the FAQ for frequently asked questions. |
|
@Sunshow Thank you for signing the Contributor License Agreement! |
|
@Sunshow thanks ! any chance you can add a test? |
|
@simonbasle just commit a new test case named |
There was a problem hiding this comment.
looking good. one last comment around tests and this is good to merge from my perspective @Sunshow 👍
… case into noOverflowAverageBigInteger()
…ic, not good side effect.
|
if that's all good from your perspective @Sunshow I'll rebase on top of 3.4.x and merge in that branch (will need to do a force-push of your branch, FYI) |
That's ok, looking forward to the new release. |
|
I cannot rework the PR, either because you haven't authorized maintainers to modify your fork/PR or because this was opened from your fork's |
|
Complete the PR. |
I'm assuming you're asking me to add the fix in 3.4.x branch myself? Yet, let me know if you still want to contribute it. In order to avoid the same confusion in the future, I would suggest always contributing PRs from feature branches (not a direct equivalent of the target maintenance branch) and making sure the "allow maintainers of the project to contribute to that PR" checkbox is checked. |
My fault, that's my first PR, and I'm busy taking care of my babies, later I will learn how to continue. |
|
that's no problem @Sunshow ! thanks for bringing the issue to our attention, in any case 🙇 👍 I will take care of that one, don't worry |
This commit improves the precision and conversion behavior of `MathFlux` `sumBigInteger` and `averageBigInteger` methods. These method better deal with fractional types by internally summing using a `BigDecimal` rather than a `BigInteger`. As a result, the only lossy step is at the end when the result is effectively presented as a `BigInteger`: - `sum`: remaining fractional part of the BigDecimal sum is dropped - `average`: the division by `count` is applied, then the result's fractional part is dropped Previously the fractional part would be progressively dropped by implicit conversion of each incoming element to `long`. Similar to precision improvement done in #260/#261 and earlier iteration done in #284.
Previous version use number.longValue, and may be out of range, use String to fix it just like BigDecimal.