My shuffle function works in singular but when combining multiple calls into one function it breaks

Question

I must be missing some basic principle here but the function shuffle works as intended however when I try and call multiple times inside another function any call after the first results in an array filled with 52 undefined entries.

function shuffle(array) {
    let newDeck = [];

    for (i=52; i>0; i--) {
        let randomPick = Math.floor((Math.random() * array.length));

        newDeck.push(array[randomPick]);
        deck.splice(randomPick, 1);
    }
    deck = newDeck;
    console.log(deck);
}

function fullShuffle(cards) {
    shuffle(cards);
    shuffle(cards);
}

or, return deck... or, pass in the deck to be shuffled... your function creates a deck... — iAmOren, Oct 22 '20 at 14:09
let deck = [ "ac", "2c", "3c", "4c", "5c", "6c", "7c", "8c", "9c", "10c", "jc", "qc", "kc", "ad", "2d", "3d", "4d", "5d", "6d", "7d", "8d", "9d", "10d", "jd", "qd", "kd", "ah", "2h", "3h", "4h", "5h", "6h", "7h", "8h", "9h", "10h", "jh", "qh", "kh", "as", "2s", "3s", "4s", "5s", "6s", "7s", "8s", "9s", "10s", "js", "qs", "ks", ]; — svedrup, Oct 22 '20 at 14:10
You remove cards from deck and read from array? Sounds fishy — epascarello, Oct 22 '20 at 14:12
https://stackoverflow.com/questions/2450954/how-to-randomize-shuffle-a-javascript-array Use that to shuffle your array, — epascarello, Oct 22 '20 at 14:13
So you have `deck`, `cards` , `newDeck`, `array` ... that are quite a lot of deck of cards. Why so many? And why pass an argument when you actually seem to have a global variable? Or better, why a global variable when you have a function argument, and could return the result? — trincot, Oct 22 '20 at 14:13

trincot · Accepted Answer · 2020-10-22T14:19:21.443

The code is not complete in the question, but it seems deck is a global variable, which you also pass as argument to fullShuffle. The issue is that deck.splice empties that array completely in one call of shuffle, so that if you use that deck again and it gets passed via fullShuffle to shuffle again, you are passing an empty array and array[randomPick] will be undefined for all 52 iterations (as array is the same reference as deck if my assumption about the rest of your is right).

If you want to implement your shuffle this way, then:

Don't use a global variable inside that function
Rely only on the argument that the function gets
Take a copy of that array, so that you can happily splice without affecting the array of the caller
Return the shuffled array to the caller

Code:

function shuffle(array) {
    let newDeck = [];
    array = [...array]; // take copy!

    for (i=52; i>0; i--) {
        let randomPick = Math.floor((Math.random() * array.length));

        newDeck.push(array[randomPick]);
        array.splice(randomPick, 1);
    }
    console.log(newDeck);
    return newDeck; // return it.
}

function fullShuffle(cards) {
    cards = shuffle(cards); // capture the returned, shuffled array
    cards = shuffle(cards);
    return cards; // return to caller
}

// can be called like this:

var deck = [ "ac", "2c", "3c", "4c", "5c", "6c", "7c", "8c", "9c", "10c", "jc", "qc", "kc", "ad", "2d", "3d", "4d", "5d", "6d", "7d", "8d", "9d", "10d", "jd", "qd", "kd", "ah", "2h", "3h", "4h", "5h", "6h", "7h", "8h", "9h", "10h", "jh", "qh", "kh", "as", "2s", "3s", "4s", "5s", "6s", "7s", "8s", "9s", "10s", "js", "qs", "ks" ];
deck = fullShuffle(deck);

Although for a human shuffle it makes sense to shuffle the same deck twice, for a function like this, there is little sense in calling it twice. It is like you don't trust the function to shuffle randomly enough...

Note also that there are better implementations of shuffle. Have a look here where you find implementations of the Durstenfeld algorithm.

Thanks so much, Im very new to this and wanted to create something simple with no framework. i realise the need to shuffle more than once in a function like this compared to a human shuffle makes no sense but i thought it would be fun to replicate that action in code. And in doing so i created a problem for myself and have remebered/learnt about returning resultsand assigning them to variables! — svedrup, Oct 22 '20 at 14:32

My shuffle function works in singular but when combining multiple calls into one function it breaks

1 Answers1