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

Gilles commented on MATH-1019:
------------------------------

IMHO, the svn log and the bug tracking system are not solely directed to 
library users.
Developers too might like to be able to follow why and how code changes.

I understand your concern, but "historically" MATH-1010 was not meant to fix 
something, just to move code from one place to another; I fortuitously 
uncovered the bug because the method was intended to do more than is actually 
needed in "RandomDataGenerator" ("user" behaviour was indeed fine but 
"developer" code was misleading).

In order to not frighten users, I can add a comment in the "changes.xml" file 
to that effect; i.e. for MATH-1020, something like: This bug does not affect 
applications using a previous version of Commons Math. For this issue, it 
should be obvious since the method "shuffle" was private.

                
> "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