[GitHub] [commons-numbers] aherbert commented on a diff in pull request #114: Some minors improvements.

[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