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)