commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Gilles (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (MATH-1019) "shuffle" method broken (in class "o.a.c.m.random.RandomDataGenerator")
Date Mon, 12 Aug 2013 14:10:49 GMT

    [ https://issues.apache.org/jira/browse/MATH-1019?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13736883#comment-13736883
] 

Gilles edited comment on MATH-1019 at 8/12/13 2:09 PM:
-------------------------------------------------------

bq. [...] "dead trees version" [...]

Oh, I got the expression now. ;)

But I still don't see the reference; the one in the Javadoc is not a link to a book, and since
the link is dead, it couldn't be more useless.
Then the one which you mention is a PDF, and IIUC, you said that it might not be reliable
to reference that link.

bq. The wikipedia article covers several different algorithms, one of which is similar to
what we have.

Thus we can put a link to the appropriate [section|http://en.wikipedia.org/wiki/Fisher–Yates_shuffle#The_modern_algorithm].
Also it's more user-friendly IMO to be redirected to that section than to have to download
the whole textbook and refer to a page number.

                
      was (Author: erans):
    bq. [...] "dead trees version" [...]

Oh, I got the expression now. ;)

But I still don't see the reference; the one in the Javadoc is not a link to a book, and since
the link is dead, it couldn't be more useless.
Then the one which you mention is a PDF, and IIUC, you said that it might be reliable to reference
that link.

bq. The wikipedia article covers several different algorithms, one of which is similar to
what we have.

Thus we can put a link to the appropriate [section|http://en.wikipedia.org/wiki/Fisher–Yates_shuffle#The_modern_algorithm].

                  
> "shuffle" method broken (in class "o.a.c.m.random.RandomDataGenerator")
> -----------------------------------------------------------------------
>
>                 Key: MATH-1019
>                 URL: https://issues.apache.org/jira/browse/MATH-1019
>             Project: Commons Math
>          Issue Type: Bug
>            Reporter: Gilles
>             Fix For: 3.3
>
>
> The method does not abide by its contract: elements before the "end" index are included
in the shuffle.
> {code}
> /**
>  * Uses a 2-cycle permutation shuffle to randomly re-order the last elements 
>  * of list.
>  *
>  * @param list list to be shuffled
>  * @param end element past which shuffling begins
>  */
> private void shuffle(int[] list, int end) {
>     int target = 0;
>     for (int i = list.length - 1; i >= end; i--) {
>         if (i == 0) { // XXX "0" should be "end"
>             target = 0; // XXX "0" should be "end"
>         } else {
>             // NumberIsTooLargeException cannot occur
>             target = nextInt(0, i); // XXX "0" should be "end"
>         }
>         int temp = list[target];
>         list[target] = list[i];
>         list[i] = temp;
>     }
> }
> {code}
> I'm going to introduce the above corrections in the new implementation to be located
in "MathArrays" (cf. issue MATH-1010).

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Mime
View raw message