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

Phil Steitz commented on MATH-1019:
-----------------------------------

It might be better not to log this and MATH-1020 as separate issues - i.e., 
this is just work related to making the formerly private internal method 
public.  As you note, the bugged API spec / impl does not impact its use in 
RandomDataGenerator.  Tracking this and MATH-1020 as issues against 3.2 and 
reporting their resolution in 3.3 may be misleading to users reading the 
release notes.  The 3.2 implementation of nextPermutation in 
RandomDataGenerator performs according to spec and seeing the bug report and 
resolution in the 3.3 release notes may give users the impression that there is 
an actual usage-impacting bug in the 3.2 impl, which is not true.  May be best 
to just note the problem in the other issue (MATH-1010) and drop this and 
MATH-1020 as separate issues.
                
> "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
>    Affects Versions: 3.2
>            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

Reply via email to