0

I am trying to implement a shuffled deck of cards in Javascript, but I am running into behavior I am having a hard time understanding.

Here is my code. For conciseness, this deck contains only cards with faces 1-5 and only two suits:

function shuffleArray(array) {
    for (var i = array.length - 1; i > 0; i--) {
        var j = Math.floor(Math.random() * (i + 1));
        var temp = array[i];
        array[i] = array[j];
        array[j] = temp;
    }
}

function getDeck() {
  let values = Array(5).fill().map((element, index) => index + 1);
  let suits = ['H', 'D'];
  let deck = new Array();

  values.forEach(function(value) {
    suits.forEach(function(suit) {
      let card = {
        'value': value,
        'suit': suit
      }
      deck.push(card);
    })
  })
  return deck
}

var deck = getDeck();
console.log(deck);
shuffleArray(deck);
console.log(deck);

I took the shuffle in place code from this answer

I would expect for the first log statement to show an unshuffled deck and the second log statement to show a shuffled deck. Instead, they both show identically shuffled decks! It's as if the randomization is reaching backwards in time!

enter image description here

To make matters worse, when I was first trying to code a minimal example, I didn't bother with values and suits and tried only to shuffle a small array, but in that case, it worked exactly as expected, only deepening my confusion...

function shuffleArray(array) {
    for (var i = array.length - 1; i > 0; i--) {
        var j = Math.floor(Math.random() * (i + 1));
        var temp = array[i];
        array[i] = array[j];
        array[j] = temp;
    }
}

function getDeck() {
    let deck = [1, 2, 3, 4, 5];
    return deck;
}


var deck = getDeck();
console.log(deck);
shuffleArray(deck);
console.log(deck);

In that case, the output is

enter image description here

What's going on? Why does the simple case work but the more complex case not seem to? What about timing or scoping am I misunderstanding?

HarlandMason
  • 779
  • 5
  • 17
  • 2
    `console.log` is not synchronous. If you need to log an array as it exists _at that exact moment in time_, log `array.slice()` instead of `array`. – Mike 'Pomax' Kamermans Apr 27 '22 at 15:45
  • Chrome devtools doesn't show you the contents of the array until you click it in the console, but at that point it's already been changed by shuffleArray function. You could console.log(JSON.stringify(deck)) – James Apr 27 '22 at 15:45

1 Answers1

1

You're modifying directly on the main array (the original deck) and console.log is not synchronous, so that is why both are the same. I'd suggest you clone another array and modify that new array instead

const newArray = [...array] //clone the original array to the new array

function shuffleArray(array) {
    const newArray = [...array]
    for (var i = newArray.length - 1; i > 0; i--) {
        var j = Math.floor(Math.random() * (i + 1));
        var temp = newArray[i];
        newArray[i] = newArray[j];
        newArray[j] = temp;
    }
    return newArray
}

function getDeck() {
  let values = Array(5).fill().map((element, index) => index + 1);
  let suits = ['H', 'D'];
  let deck = new Array();

  values.forEach(function(value) {
    suits.forEach(function(suit) {
      let card = {
        'value': value,
        'suit': suit
      }
      deck.push(card);
    })
  })
  return deck
}

var deck = getDeck();
console.log(deck);
const newDeck = shuffleArray(deck);
console.log(newDeck);
.as-console-wrapper { max-height: 100% !important; top: 0; }
Nick Vu
  • 14,512
  • 4
  • 21
  • 31
  • I understand the modification is being done in place, the question is why does it seem to be happening before the function is called. The simpler example with just the five element array shows the behavior I expect. I'm trying to understand what makes the five element array case function differently from the more complex case at the start of the post – HarlandMason Apr 27 '22 at 15:48
  • ah, I didn't catch that part in your question, but seemingly, you're facing that problem with `console.log` because it's not synchronous (I just mentioned this part in the answer). By the way, we should avoid modifying the original deck too because you may reuse it for a reset behaviour or further modification with another logic. @HarlandMason – Nick Vu Apr 27 '22 at 15:50