[
https://issues.apache.org/jira/browse/RNG-61?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16691423#comment-16691423
]
ASF GitHub Bot commented on RNG-61:
-----------------------------------
GitHub user aherbert opened a pull request:
https://github.com/apache/commons-rng/pull/13
RNG-61: Remove unnecessary conditional from inside shuffle loop
I have also updated the constructor exception messages to be clearer on
what has failed and added a Javadoc link to a definition of Permutation.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/aherbert/commons-rng improvement-RNG-61
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/commons-rng/pull/13.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #13
----
commit d3ac7f75e2ed7e21fc48c5db8c5ea8bfbe3533f3
Author: Alex Herbert <a.herbert@...>
Date: 2018-11-19T08:54:17Z
RNG-61: Remove unnecessary conditional from inside shuffle loop
----
> PermutationSampler shuffle contains unnecessary conditional
> -----------------------------------------------------------
>
> Key: RNG-61
> URL: https://issues.apache.org/jira/browse/RNG-61
> Project: Commons RNG
> Issue Type: Improvement
> Components: sampling
> Reporter: Alex D Herbert
> Priority: Trivial
> Fix For: 1.2
>
>
> The Fisher-Yates algorithm in the PermutationSampler has a conditional {{if}}
> statement within the loop that checks if the step is the first in the loop.
> It then swaps a position with itself. For example for the towards head
> variant:
> {code:java}
> for (int i = 0; i <= start; i++) {
> final int target;
> // Check for start
> if (i == start) {
> target = start; // this means target == i
> } else {
> target = rng.nextInt(start - i + 1) + i;
> }
> // if target == i then this step is not needed.
> final int temp = list[target];
> list[target] = list[i];
> list[i] = temp;
> }
> {code}
> This can be removed by altering the iteration loop:
> {code:java}
> for (int i = start; i > 0; i--) {
> final int target = rng.nextInt(i + 1);
> final int temp = list[target];
> list[target] = list[i];
> list[i] = temp;
> }
> {code}
> An equivalent fix is available for the shuffle towards the tail.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
