Javascript shuffle function returns same value in for loop

Question

Why do I get three arrays in the same order while I shuffle them with a shuffle function in a for loop?

var items = ['x', 'y', 'z', 'a', 'b'];
var createSlots = function(slots) 
{
    slots = slots || 3;
    var slotRack = [];
    for (var i = 0; i < slots; i++ )
    {
        slotRack.push(shuffle(items));
    }
    return slotRack;
}

function shuffle(o){ //v1.0
    for(var j, x, i = o.length; i; j = Math.floor(Math.random() * i), x = o[--i], o[i] = o[j], o[j] = x);
    return o;
};

var slotmachine = createSlots();  

// returns three arrays with values in the same order... do not want that... :(
console.log(slotmachine);

You're always mutating the same Array object. You need to make a copy if you want each result to hold a unique set of values. `slotRack.push(shuffle(items.slice())` — , Oct 28 '14 at 15:08
Also see: http://en.wikipedia.org/wiki/Fisher%E2%80%93Yates_shuffle — Matt Burland, Oct 28 '14 at 15:21

score 2 · Accepted Answer · edited May 23 '17 at 11:49

2

squint stated your problem in a comment above.

Anyway, here's a cooler shuffle method that will always get you out of trouble:

function shuffle(arr) {
    return arr.sort(function () {
        return Math.random() - Math.random()
    });
};

EDIT (thank's Mr. Llama):

Use the Fisher-Yates shuffle(thanks to Christoph for the implementation) instead:

function shuffle(array) {
    var tmp, current, top = array.length;

    if(top) while(--top) {
        current = Math.floor(Math.random() * (top + 1));
        tmp = array[current];
        array[current] = array[top];
        array[top] = tmp;
    }

    return array;
}

edited May 23 '17 at 11:49

Community

1
1

answered Oct 28 '14 at 15:08

Amir Popovich

29,350
9
53
99

1

Nonononono, don't *ever* use `sort` with `Math.random()` to shuffle an array. The result [**is not random**](http://sroucheray.org/blog/2009/11/array-sort-should-not-be-used-to-shuffle-an-array/). Use the [Fisher-Yates shuffle](http://stackoverflow.com/a/962829/477563) instead. – Mr. Llama Oct 28 '14 at 15:15
@Mr.Llama - Didn't know that - Thanks for you referral. I also updated my post to prevent future misunderstandings. – Amir Popovich Oct 28 '14 at 15:21

score 1 · Answer 2 · answered Oct 28 '14 at 15:09

1

You are pushing in a reference to the same array in every iteration of the loop, try this instead:

 slotRack.push(shuffle(items.slice()));

Check it out here: JSFiddle

Edit: Maybe its better to do the slice() in your function, so return o.slice() and you don't have to worry about it when using the function.

answered Oct 28 '14 at 15:09

Johan Karlsson

6,419
1
19
28

Thanks guys, is using the slice() function to lose the reference a common practice? – Feddman Oct 28 '14 at 15:24

Javascript shuffle function returns same value in for loop

2 Answers2