Shuffle deck of cards in JavaScript

Question

I know randomness in programming can be a deep topic, and searching the topic seems to result in Fisher-Yates being the best method, but I'm curious what people would say about this method.

Basically, since I have the cards as objects, I just add a property of a random number then sort the entire array based on those random numbers.

const suits = [
  {
    digit: 'H',
    word: 'hearts'
  },
  {
    digit: 'C',
    word: 'clubs'
  },
  {
    digit: 'D',
    word: 'diamonds'
  },
  {
    digit: 'S',
    word: 'spades'
  }
]
const cardsWithoutSuits = [
  {
    numeric: 1,
    word: 'ace',
    digit: 'A'
  },
  {
    numeric: 2,
    word: 'two',
  },
  {
    numeric: 3,
    word: 'three',
  },
  {
    numeric: 4,
    word: 'four',
  },
  {
    numeric: 5,
    word: 'five',
  },
  {
    numeric: 6,
    word: 'six',
  },
  {
    numeric: 7,
    word: 'seven',
  },
  {
    numeric: 8,
    word: 'eight',
  },
  {
    numeric: 9,
    word: 'nine',
  },
  {
    numeric: 10,
    word: 'ten',
  },
  {
    numeric: 11,
    word: 'jack',
    digit: 'J'
  },
  {
    numeric: 12,
    word: 'queen',
    digit: 'Q'
  },
  {
    numeric: 13,
    word: 'king',
    digit: 'K'
  }
]

function createDeck(decks = 1){
  let deck = [];
  for (let i = 0; i < decks; i++) {
    suits.forEach( x => {
      cardsWithoutSuits.forEach( y => {
        deck.push({
          numeric: y.numeric,
          word: y.word,
          suit: x.word,
          phrase: `${y.word} of ${x.word}`,
          abbr: `${y.hasOwnProperty('digit') ? y.digit : y.numeric}${x.digit}`
        })
      })
    })
  }
  return deck;
}

function shuffle(array){
  array.forEach( x =>{
    x.ran = Math.random();
  })
  array.sort( (a, b) =>{
    return a.ran - b.ran;
  })

  return array;
}

let deck = shuffle(createDeck(2));
console.log(deck);

Answer 1

There has already been some discussion by @Oh My Goodness and @Blindman67 in the comments about the time complexity of this code vs Fisher-Yates. I don't really have much to add to that discussion but will offer some suggestions about the code on a deeper level below.

Like I mentioned in my answer to your post Simple math game in JavaScript some variable names could more accurately hint at the semantic meaning of the value - for example, instead of x.ran = Math.random();, a name like sortVal would hint that the value is to be used for sorting.

array.forEach( x =>{
  x.sortVal = Math.random();
})

---

The arrow function in the shuffle() method is quite short and thus can be simplified to a single line:

array.sort( (a, b) => a.sortVal - b.sortVal)

The forEach block above it could also be simplified:

array.forEach( x => x.sortVal = Math.random())

In this case, x.sortVal would be returned in each iteration but that doesn't make a difference.

---

The Array sort method Array.prototype.sort() returns the array so the shuffle() function could be simplified to have the return statement on the same line as the call to that method:

  return array.sort( (a, b) => a.sortVal - b.sortVal)

---

Additionally, any value that doesn't get re-assigned could be declared using const, which would help avoid any unintentional re-assignment - e.g. deck within shuffle(), and deck at the end of the code.