You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@groovy.apache.org by GitBox <gi...@apache.org> on 2020/11/30 13:08:50 UTC
[GitHub] [groovy] moonfruit opened a new pull request #1437: Avoid toString() for converting integer types to BigInteger and BigDecimal
moonfruit opened a new pull request #1437:
URL: https://github.com/apache/groovy/pull/1437
Avoid toString() for converting integer types to BigInteger and BigDecimal.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [groovy] moonfruit commented on pull request #1437: Avoid toString() for converting integer types to BigInteger and BigDecimal
Posted by GitBox <gi...@apache.org>.
moonfruit commented on pull request #1437:
URL: https://github.com/apache/groovy/pull/1437#issuecomment-736213302
Thank you!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [groovy] moonfruit commented on pull request #1437: Avoid toString() for converting integer types to BigInteger and BigDecimal
Posted by GitBox <gi...@apache.org>.
moonfruit commented on pull request #1437:
URL: https://github.com/apache/groovy/pull/1437#issuecomment-736278776
@danielsun1106 For 6f08fd0, convert from `BigDecimal` to `BigInteger`, better use `((BigDecimal) n).toBigIntegerExact()` instead of `new BigInteger(((BigDecimal) n).toPlainString())`.
Should I make another pull request?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [groovy] moonfruit commented on pull request #1437: Avoid toString() for converting integer types to BigInteger and BigDecimal
Posted by GitBox <gi...@apache.org>.
moonfruit commented on pull request #1437:
URL: https://github.com/apache/groovy/pull/1437#issuecomment-736194622
@danielsun1106 Should I move Integer and Long to the beginning of the conditions?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [groovy] danielsun1106 commented on pull request #1437: Avoid toString() for converting integer types to BigInteger and BigDecimal
Posted by GitBox <gi...@apache.org>.
danielsun1106 commented on pull request #1437:
URL: https://github.com/apache/groovy/pull/1437#issuecomment-736297322
@moonfruit Nice catch. New PR is welcome ;-)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [groovy] danielsun1106 commented on a change in pull request #1437: Avoid toString() for converting integer types to BigInteger and BigDecimal
Posted by GitBox <gi...@apache.org>.
danielsun1106 commented on a change in pull request #1437:
URL: https://github.com/apache/groovy/pull/1437#discussion_r532598636
##########
File path: src/main/java/org/codehaus/groovy/runtime/typehandling/NumberMath.java
##########
@@ -168,11 +168,26 @@ public static boolean isBigInteger(Number number) {
}
public static BigDecimal toBigDecimal(Number n) {
- return (n instanceof BigDecimal ? (BigDecimal) n : new BigDecimal(n.toString()));
+ if (n instanceof BigDecimal) {
+ return (BigDecimal) n;
+ }
+ if (n instanceof BigInteger) {
+ return new BigDecimal((BigInteger) n);
+ }
+ if (n instanceof Byte || n instanceof Short || n instanceof Integer || n instanceof Long) {
Review comment:
I suggest to put Integer and Long at the beginning to avoid checks as possible as we could, because we use Integer and Long more often in the life 😉
##########
File path: src/main/java/org/codehaus/groovy/runtime/typehandling/NumberMath.java
##########
@@ -168,11 +168,26 @@ public static boolean isBigInteger(Number number) {
}
public static BigDecimal toBigDecimal(Number n) {
- return (n instanceof BigDecimal ? (BigDecimal) n : new BigDecimal(n.toString()));
+ if (n instanceof BigDecimal) {
+ return (BigDecimal) n;
+ }
+ if (n instanceof BigInteger) {
+ return new BigDecimal((BigInteger) n);
+ }
+ if (n instanceof Byte || n instanceof Short || n instanceof Integer || n instanceof Long) {
+ return new BigDecimal(n.longValue());
+ }
+ return new BigDecimal(n.toString());
}
public static BigInteger toBigInteger(Number n) {
- return (n instanceof BigInteger ? (BigInteger) n : new BigInteger(n.toString()));
+ if (n instanceof BigInteger) {
+ return (BigInteger) n;
+ }
+ if (n instanceof Byte || n instanceof Short || n instanceof Integer || n instanceof Long) {
Review comment:
Ditto
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [groovy] asfgit closed pull request #1437: Avoid toString() for converting integer types to BigInteger and BigDecimal
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #1437:
URL: https://github.com/apache/groovy/pull/1437
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [groovy] danielsun1106 commented on pull request #1437: Avoid toString() for converting integer types to BigInteger and BigDecimal
Posted by GitBox <gi...@apache.org>.
danielsun1106 commented on pull request #1437:
URL: https://github.com/apache/groovy/pull/1437#issuecomment-735813200
Merged. Thanks!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [groovy] danielsun1106 commented on pull request #1437: Avoid toString() for converting integer types to BigInteger and BigDecimal
Posted by GitBox <gi...@apache.org>.
danielsun1106 commented on pull request #1437:
URL: https://github.com/apache/groovy/pull/1437#issuecomment-736202799
Your PR has been merged :-)
https://github.com/apache/groovy/commit/8af30255c45d9adb6f80590d6ca13885d1b338d7
and I've tweaked it to catch the release train:
https://github.com/apache/groovy/commit/dab8f4e1f277701948db1e56d19ee1ecb8861051
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org