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 2022/06/26 20:41:44 UTC
[GitHub] [commons-numbers] arturobernalg opened a new pull request, #114: Some minors improvements.
arturobernalg opened a new pull request, #114:
URL: https://github.com/apache/commons-numbers/pull/114
* Use java8 Comparator.
* Use Math.min
* Use System.arraycopy
--
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.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-numbers] aherbert commented on a diff in pull request #114: Some minors improvements.
Posted by GitBox <gi...@apache.org>.
aherbert commented on code in PR #114:
URL: https://github.com/apache/commons-numbers/pull/114#discussion_r906875202
##########
commons-numbers-arrays/src/main/java/org/apache/commons/numbers/arrays/SortInPlace.java:
##########
@@ -40,7 +40,7 @@
*/
public enum SortInPlace {
/** Sort in ascending order. */
- ASCENDING((o1, o2) -> Double.compare(o1.key(), o2.key())),
+ ASCENDING(Comparator.comparingDouble(PairDoubleInteger::key)),
Review Comment:
I think the current code it is very clear that the only difference is that they use Double.compare with the keys in different order. Note in the commit history the original version was:
```Java
ASCENDING((o1, o2) -> Double.compare(o1.key(), o2.key())),
DESCENDING(ASCENDING.comparator.reversed());
```
This was changed to help clarify the difference. It also reduces the abstraction of the comparator with function references which may have a performance impact.
##########
commons-numbers-combinatorics/src/main/java/org/apache/commons/numbers/combinatorics/LogFactorial.java:
##########
@@ -55,15 +55,14 @@ private LogFactorial(int numValues,
int endCopy;
if (cache == null || cache.length <= beginCopy) {
endCopy = beginCopy;
- } else if (cache.length <= numValues) {
- endCopy = cache.length;
} else {
- endCopy = numValues;
+ endCopy = Math.min(cache.length, numValues);
}
+
// Copy available values.
- for (int i = beginCopy; i < endCopy; i++) {
- logFactorials[i] = cache[i];
+ if (endCopy - beginCopy > 0) {
+ System.arraycopy(cache, beginCopy, logFactorials, beginCopy, endCopy - 2);
Review Comment:
Should this be: length = endCopy - beginCopy, i.e.
```Java
System.arraycopy(cache, beginCopy, logFactorials, beginCopy, endCopy - beginCopy);
```
--
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.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-numbers] codecov-commenter commented on pull request #114: Some minors improvements.
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #114:
URL: https://github.com/apache/commons-numbers/pull/114#issuecomment-1166640153
# [Codecov](https://codecov.io/gh/apache/commons-numbers/pull/114?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#114](https://codecov.io/gh/apache/commons-numbers/pull/114?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (52d4484) into [master](https://codecov.io/gh/apache/commons-numbers/commit/f31cf16cff3d7462ca4bad3c0240ba5935d94785?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f31cf16) will **decrease** coverage by `0.00%`.
> The diff coverage is `100.00%`.
```diff
@@ Coverage Diff @@
## master #114 +/- ##
============================================
- Coverage 99.12% 99.12% -0.01%
+ Complexity 1654 1652 -2
============================================
Files 64 64
Lines 4227 4225 -2
Branches 835 834 -1
============================================
- Hits 4190 4188 -2
Misses 10 10
Partials 27 27
```
| [Impacted Files](https://codecov.io/gh/apache/commons-numbers/pull/114?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...org/apache/commons/numbers/arrays/SortInPlace.java](https://codecov.io/gh/apache/commons-numbers/pull/114/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9ucy1udW1iZXJzLWFycmF5cy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvY29tbW9ucy9udW1iZXJzL2FycmF5cy9Tb3J0SW5QbGFjZS5qYXZh) | `100.00% <100.00%> (ø)` | |
| [...he/commons/numbers/combinatorics/LogFactorial.java](https://codecov.io/gh/apache/commons-numbers/pull/114/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9ucy1udW1iZXJzLWNvbWJpbmF0b3JpY3Mvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2NvbW1vbnMvbnVtYmVycy9jb21iaW5hdG9yaWNzL0xvZ0ZhY3RvcmlhbC5qYXZh) | `100.00% <100.00%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/commons-numbers/pull/114?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/commons-numbers/pull/114?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [f31cf16...52d4484](https://codecov.io/gh/apache/commons-numbers/pull/114?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-numbers] aherbert merged pull request #114: Some minors improvements.
Posted by GitBox <gi...@apache.org>.
aherbert merged PR #114:
URL: https://github.com/apache/commons-numbers/pull/114
--
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.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org