[GitHub] [commons-numbers] arturobernalg commented on a change in pull request #99: Replace manual copying of array contents by calls to System.arraycopy().

[GitBox <gi...@apache.org>]

arturobernalg commented on a change in pull request #99:
URL: https://github.com/apache/commons-numbers/pull/99#discussion_r654891918



##########
File path: commons-numbers-combinatorics/src/main/java/org/apache/commons/numbers/combinatorics/LogFactorial.java
##########
@@ -62,9 +62,7 @@ private LogFactorial(int numValues,
         }
 
         // Copy available values.
-        for (int i = beginCopy; i < endCopy; i++) {
-            logFactorials[i] = cache[i];
-        }
+        if (endCopy - 2 >= 0) System.arraycopy(cache, beginCopy, logFactorials, beginCopy, endCopy - 2);

Review comment:
       True. Changed. TY

##########
File path: commons-numbers-combinatorics/src/main/java/org/apache/commons/numbers/combinatorics/FactorialDouble.java
##########
@@ -64,9 +64,7 @@ private FactorialDouble(int numValues,
         }
 
         // Copy available values.
-        for (int i = beginCopy; i < endCopy; i++) {
-            factorialsDouble[i] = cache[i];
-        }
+        if (endCopy - 2 >= 0) System.arraycopy(cache, beginCopy, factorialsDouble, beginCopy, endCopy - 2);

Review comment:
       True. Changed. TY




-- 
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

[Numbers][All] Code readability vs System.arraycopy (Was: [...] pull request #99: [...])

[Gilles Sadowski <gi...@gmail.com>]

Hi.

FTR: I don't agree with such changes when they obfuscate the code
for no measurable gain (until proven otherwise[1]).
If the class is used properly, the "manual" copying is a minute part of
usage duration; if not used properly (e.g. calling "create()" instead of
"withCache()" on an existing instance), the "saved" copying time will
be marginal wrt the lost time due to repeating the other (likely much
lengthier) computations.

Regards,
Gilles

[1] Through JMH benchmarks _on the use-case at hand_.

Le dim. 20 juin 2021 à 09:35, GitBox <gi...@apache.org> a écrit :
>
>
> arturobernalg commented on a change in pull request #99:
> URL: https://github.com/apache/commons-numbers/pull/99#discussion_r654891918
>
>
>
> ##########
> File path: commons-numbers-combinatorics/src/main/java/org/apache/commons/numbers/combinatorics/LogFactorial.java
> ##########
> @@ -62,9 +62,7 @@ private LogFactorial(int numValues,
>          }
>
>          // Copy available values.
> -        for (int i = beginCopy; i < endCopy; i++) {
> -            logFactorials[i] = cache[i];
> -        }
> +        if (endCopy - 2 >= 0) System.arraycopy(cache, beginCopy, logFactorials, beginCopy, endCopy - 2);
>
> Review comment:
>        True. Changed. TY
>
> ##########
> File path: commons-numbers-combinatorics/src/main/java/org/apache/commons/numbers/combinatorics/FactorialDouble.java
> ##########
> @@ -64,9 +64,7 @@ private FactorialDouble(int numValues,
>          }
>
>          // Copy available values.
> -        for (int i = beginCopy; i < endCopy; i++) {
> -            factorialsDouble[i] = cache[i];
> -        }
> +        if (endCopy - 2 >= 0) System.arraycopy(cache, beginCopy, factorialsDouble, beginCopy, endCopy - 2);
>
> Review comment:
>        True. Changed. TY
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org