You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "Gilles (JIRA)" <ji...@apache.org> on 2018/11/16 12:39:01 UTC
[jira] [Commented] (RNG-61) PermutationSampler shuffle contains
unnecessary conditional
[ https://issues.apache.org/jira/browse/RNG-61?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16689369#comment-16689369 ]
Gilles commented on RNG-61:
---------------------------
Thanks. PR? :)
> 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
> Affects Versions: 1.2
> Reporter: Alex D Herbert
> Priority: Trivial
>
> 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)