You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2020/08/25 17:36:14 UTC
[GitHub] [commons-numbers] XenoAmess opened a new pull request #84: [NUMBERS-151] performance refine for ArithmeticUtils.pow
XenoAmess opened a new pull request #84:
URL: https://github.com/apache/commons-numbers/pull/84
----------------------------------------------------------------
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] [commons-numbers] coveralls edited a comment on pull request #84: [NUMBERS-151] performance refine for ArithmeticUtils.pow
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #84:
URL: https://github.com/apache/commons-numbers/pull/84#issuecomment-680212902
[](https://coveralls.io/builds/33004730)
Coverage increased (+0.005%) to 99.636% when pulling **4d35d71e671843519e0894557cc1f9dccc1b192e on XenoAmess:refine_ArithmeticUtils_pow** into **3282fb87ebcc905ef1daa6a7ac24740c3f73cf4f on apache:master**.
----------------------------------------------------------------
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] [commons-numbers] coveralls commented on pull request #84: [NUMBERS-151] performance refine for ArithmeticUtils.pow
Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #84:
URL: https://github.com/apache/commons-numbers/pull/84#issuecomment-680212902
[](https://coveralls.io/builds/32994925)
Coverage decreased (-0.1%) to 99.488% when pulling **13ad4e1e869e5fa4c6b2df95d1419fc93e8cd817 on XenoAmess:refine_ArithmeticUtils_pow** into **3282fb87ebcc905ef1daa6a7ac24740c3f73cf4f on apache:master**.
----------------------------------------------------------------
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] [commons-numbers] coveralls edited a comment on pull request #84: [NUMBERS-151] performance refine for ArithmeticUtils.pow
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #84:
URL: https://github.com/apache/commons-numbers/pull/84#issuecomment-680212902
[](https://coveralls.io/builds/33008473)
Coverage increased (+0.002%) to 99.636% when pulling **8e185bd93942950b6cb1075598ad55b8d41005e4 on XenoAmess:refine_ArithmeticUtils_pow** into **c3d173ae9cf25329644b87b8e064f56a2c31ea53 on apache:master**.
----------------------------------------------------------------
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] [commons-numbers] XenoAmess commented on a change in pull request #84: [NUMBERS-151] performance refine for ArithmeticUtils.pow
Posted by GitBox <gi...@apache.org>.
XenoAmess commented on a change in pull request #84:
URL: https://github.com/apache/commons-numbers/pull/84#discussion_r477166070
##########
File path: commons-numbers-core/src/main/java/org/apache/commons/numbers/core/ArithmeticUtils.java
##########
@@ -266,6 +266,16 @@ public static long lcm(long a, long b) {
/**
* Raise an int to an int power.
*
+ * <p>Special cases:</p>
Review comment:
@aherbert Ah, I forgot it :)
will do now.
----------------------------------------------------------------
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] [commons-numbers] aherbert commented on pull request #84: [NUMBERS-151] performance refine for ArithmeticUtils.pow
Posted by GitBox <gi...@apache.org>.
aherbert commented on pull request #84:
URL: https://github.com/apache/commons-numbers/pull/84#issuecomment-680243977
You have added no tests. There are a few lines not covered by the existing tests.
----------------------------------------------------------------
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] [commons-numbers] asfgit merged pull request #84: [NUMBERS-151] performance refine for ArithmeticUtils.pow
Posted by GitBox <gi...@apache.org>.
asfgit merged pull request #84:
URL: https://github.com/apache/commons-numbers/pull/84
----------------------------------------------------------------
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] [commons-numbers] XenoAmess commented on pull request #84: [NUMBERS-151] performance refine for ArithmeticUtils.pow
Posted by GitBox <gi...@apache.org>.
XenoAmess commented on pull request #84:
URL: https://github.com/apache/commons-numbers/pull/84#issuecomment-680263178
@aherbert
tests added.
----------------------------------------------------------------
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] [commons-numbers] aherbert commented on pull request #84: [NUMBERS-151] performance refine for ArithmeticUtils.pow
Posted by GitBox <gi...@apache.org>.
aherbert commented on pull request #84:
URL: https://github.com/apache/commons-numbers/pull/84#issuecomment-680710174
`0^0` is undefined. Here we support returning 1 as described in this Wikipedia article:
[Zero_to_the_power_of_zero](https://en.wikipedia.org/wiki/Zero_to_the_power_of_zero)
Perhaps add a comment:
```
<p>Special cases:
<ul>
<li>{@code k^0} returns {@code 1} (including {@code k=0})
<li>{@code k^1} returns {@code k} (including {@code k=0})
<li>{@code 0^0} returns {@code 1}
<li>{@code 0^e} returns {@code 0}
<li>{@code 1^e} returns {@code 1}
<li>{@code (-1)^e} returns {@code -1 or 1} if {@code e} is odd or even
</ul>
```
----------------------------------------------------------------
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] [commons-numbers] XenoAmess commented on pull request #84: [NUMBERS-151] performance refine for ArithmeticUtils.pow
Posted by GitBox <gi...@apache.org>.
XenoAmess commented on pull request #84:
URL: https://github.com/apache/commons-numbers/pull/84#issuecomment-680259161
@aherbert
> You have added no tests. There are a few lines not covered by the existing tests.
I will go see the coverage and add tests.
BTW.
Should we really throw no exception when we invoke pow(0, 0) here?
It seems...weird...
----------------------------------------------------------------
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] [commons-numbers] coveralls edited a comment on pull request #84: [NUMBERS-151] performance refine for ArithmeticUtils.pow
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #84:
URL: https://github.com/apache/commons-numbers/pull/84#issuecomment-680212902
[](https://coveralls.io/builds/33008139)
Coverage increased (+0.005%) to 99.636% when pulling **9a609f163c2bc57544c98aa8f6a4a712d23cc85c on XenoAmess:refine_ArithmeticUtils_pow** into **3282fb87ebcc905ef1daa6a7ac24740c3f73cf4f on apache:master**.
----------------------------------------------------------------
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] [commons-numbers] XenoAmess commented on a change in pull request #84: [NUMBERS-151] performance refine for ArithmeticUtils.pow
Posted by GitBox <gi...@apache.org>.
XenoAmess commented on a change in pull request #84:
URL: https://github.com/apache/commons-numbers/pull/84#discussion_r477167948
##########
File path: commons-numbers-core/src/test/java/org/apache/commons/numbers/core/ArithmeticUtilsTest.java
##########
@@ -435,6 +435,25 @@ void testIsPowerOfTwo() {
}
}
+ @Test
+ void testPowOptimize() {
Review comment:
@aherbert done.
----------------------------------------------------------------
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] [commons-numbers] coveralls edited a comment on pull request #84: [NUMBERS-151] performance refine for ArithmeticUtils.pow
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #84:
URL: https://github.com/apache/commons-numbers/pull/84#issuecomment-680212902
[](https://coveralls.io/builds/33010157)
Coverage increased (+0.002%) to 99.636% when pulling **42ebc644ff46e0cef75a59512390c9b78fa16a04 on XenoAmess:refine_ArithmeticUtils_pow** into **c3d173ae9cf25329644b87b8e064f56a2c31ea53 on apache:master**.
----------------------------------------------------------------
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] [commons-numbers] aherbert commented on a change in pull request #84: [NUMBERS-151] performance refine for ArithmeticUtils.pow
Posted by GitBox <gi...@apache.org>.
aherbert commented on a change in pull request #84:
URL: https://github.com/apache/commons-numbers/pull/84#discussion_r477145593
##########
File path: commons-numbers-core/src/main/java/org/apache/commons/numbers/core/ArithmeticUtils.java
##########
@@ -266,6 +266,16 @@ public static long lcm(long a, long b) {
/**
* Raise an int to an int power.
*
+ * <p>Special cases:</p>
Review comment:
You should put the same comment on the `long` version of the pow method.
----------------------------------------------------------------
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] [commons-numbers] aherbert commented on a change in pull request #84: [NUMBERS-151] performance refine for ArithmeticUtils.pow
Posted by GitBox <gi...@apache.org>.
aherbert commented on a change in pull request #84:
URL: https://github.com/apache/commons-numbers/pull/84#discussion_r477112917
##########
File path: commons-numbers-core/src/test/java/org/apache/commons/numbers/core/ArithmeticUtilsTest.java
##########
@@ -435,6 +435,25 @@ void testIsPowerOfTwo() {
}
}
+ @Test
+ void testPowOptimize() {
Review comment:
This should be moved above the `testIsPowerOfTwo` method. I would rename it to `testPowEdgeCases`.
----------------------------------------------------------------
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