Array occasionally returning duplicate values

Question

I am trying to make a card game that is similar to the game War. The game has two players, each with their own deck they play from.
I have a function that passes in an empty array that is the deck each player has.
This array is populated by half of the items in another array that is the entire deck.
Most of the time, this works correctly, but sometimes, the array will have 26 values, other times, some values will be duplicated.
My question is, how do I stop duplicates being passed into the player's deck array?

Player = function(wonRound, currentCards, newCards) {
  this.wonRound = wonRound;
  this.currentCards = currentCards;
  this.newCards = newCards;
}

Deck = {
  suits: ["Clubs", "Diamonds", "Hearts", "Spades"],
  cards: ["Ace", "King", "Queen", "Jack", "10", "9", "8", "7", "6", "5", "4", "3", "2"],
  deck: [],
  shuffledDeck: [],
  BuildDeck: function() {
    //Builds the deck
    for (var suit = 0; suit < this.suits.length; suit++) {
      for (var card = 0; card < this.cards.length; card++) {
        this.deck.push([this.cards[card], this.suits[suit]]);
      }
    }
    return this.deck;
  },
  ShuffleDeck: function() {
    //Shuffles the deck
    for (var card = 0; card < this.deck.length; card++) {
      this.shuffledDeck.push(this.deck[Math.floor(Math.random() * this.deck.length)]);
    }
    return this.shuffledDeck;
  },
  DistributeCards: function(playerDeck) {
    //Distributes half the deck to each player
    for (var i = 0; i < this.shuffledDeck.length / 2; i++) {
      playerDeck.push(this.shuffledDeck[i]);
    }
    return playerDeck;
  }
}
Player1 = new Player(false, [], []);
Player2 = new Player(false, [], []);
Deck.BuildDeck();
Deck.ShuffleDeck();
Deck.DistributeCards(Player1.currentCards);
for (var i = 0; i < Player1.currentCards.length; i++) {
  console.log(Player1.currentCards[i][0], Player1.currentCards[i][1], Player1.currentCards.indexOf(Player1.currentCards[i]));
}

check my answer for distributing it to second player as well :) — Dirty Developer, Feb 18 '17 at 03:07

ibrahim mahrir · Accepted Answer · 2017-02-18T02:25:40.563

You are getting duplicates because you choose from the same array without excluding the already choosen values. One way to do it is to remove (using splice) the items choosen so they won't get choosed again. Like this:

ShuffleDeck: function() {                                         // while there still items in the deck
    while(this.deck.length) {
        var index = Math.floor(Math.random() * this.deck.length); // get a random index
        this.shuffledDeck.push(this.deck.splice(index, 1)[0]);    // remove the item at index (splice will return an array so we use [0] to get the item)
    }
    return this.shuffledDeck;
},

Note: the deck will be empty afterwards, but since you are not using it again, that won't be a problem.

Edit:

Change DistributeCards to this:

DistributeCards: function(player1, player2) {
    for (var i = 0; i < this.shuffledDeck.length / 2; i++) {
        player1.push(this.shuffledDeck[i]); // push the first half to player1's deck (0, 1, 2, ...)
        player2.push(this.shuffledDeck[this.shuffledDeck.length - i - 1]); // push the other half to player2's deck (len - 1, len - 2, len - 3, ...)
    }
    // no need for return if you won't use the return value (my advice is to remove return from all the function that you don't use their return value)
}

and then use it like this:

Deck.DistributeCards(Player1.currentCards, Player2.currentCards);

Thank you for explaining everything. Now the issue is what to do when there are two players. — Robert, Feb 18 '17 at 02:18

score 0 · Answer 2 · edited May 23 '17 at 12:32

0

The problem is mainly in the way you are shuffling! in the statement:

this.shuffledDeck.push(this.deck[Math.floor(Math.random() * this.deck.length)]);

the Math.floor(Math.random() * this.deck.length) logic might give the same number twice, which will duplicate a card (and it is possible more than once) which is not what is expected in your case.
So what you really need is to create a logic that will ensure no number is repeated, and there are many ways to do this, check this SO post for multiple examples.

edited May 23 '17 at 12:32

Community

1
1

answered Feb 17 '17 at 23:59

Tha'er AlAjlouni ثائر العجلوني

14,069
7
33
38

Would shuffling the cards once they're passed into each player's deck fix it? – Robert Feb 18 '17 at 00:01
1

@Robert Probably not, since the distribution would not be random. – Brandon Anzaldi Feb 18 '17 at 00:02
@Robert I totally agree with Brandon. – Tha'er AlAjlouni ثائر العجلوني Feb 18 '17 at 00:12

score 0 · Answer 3 · answered Feb 18 '17 at 00:03

0

deck.sort(function(a, b){return 0.5 - Math.random()});

Just do this on the deck and u dont even need another deck for shuffled cards.

https://www.w3schools.com/js/js_array_sort.asp there is more array methods.

answered Feb 18 '17 at 00:03

iWillBeMaster

157
10

why do you use 0.5 ? – bormat Feb 18 '17 at 00:07
1

@bormat `Math.random` will return a number between `[0, 1[`, so `0.5 - ...` will return both negative and positive number depending on `Math.random`. `sort` will swap item depending on wheter the callback returned a positive or negative number. If the return value is always positive then `sort` will either let the array as it is or reverese it. See the docs for `sort` on [**MDN**](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/sort)! – ibrahim mahrir Feb 18 '17 at 00:12
This always returns "undefined" – Robert Feb 18 '17 at 02:15

Brandon Anzaldi · Answer 4 · 2017-02-18T00:26:01.953

Your issue lies in your ShuffleDeck function. Each time you are picking a random card from the existing deck, but you are not removing that card from the pool to be picked again. You either need to keep track of the numbers that have already been picked, or remove the element from the array once it's been shuffled into the deck. Perhaps the simplest type of shuffle is a Fisher-Yates shuffle.

In addition, your distribution will always take the first half of the deck without modifying it, so running that function without removing the elements from the deck would simply give each player the same hand of cards.

Player = function(wonRound, currentCards, newCards) {
  this.wonRound = wonRound;
  this.currentCards = currentCards;
  this.newCards = newCards;
  this.getCards = function ( cards ) {
    this.currentCards = cards;
  };
}

Deck = {
  suits: ["Clubs", "Diamonds", "Hearts", "Spades"],
  cards: ["Ace", "King", "Queen", "Jack", "10", "9", "8", "7", "6", "5", "4", "3", "2"],
  deck: [],
  shuffledDeck: [],
  BuildDeck: function() {
    //Builds the deck
    for (var suit = 0; suit < this.suits.length; suit++) {
      for (var card = 0; card < this.cards.length; card++) {
        this.deck.push([this.cards[card], this.suits[suit]]);
      }
    }
    return this.deck;
  },
  ShuffleDeck: function() {
    //Shuffles the deck
    var tempDeck = this.deck.slice(0); // Clone the deck, so the original deck still exists
    while ( tempDeck.length ) {
      var newItem = tempDeck.splice( Math.floor( Math.random() * tempDeck.length ), 1); // Pull one item from the array
      this.shuffledDeck.push( newItem[0] ); // Add it to the shuffled array
    }
    return this.shuffledDeck;
  },
  DistributeCards: function( len ) {
    //Distributes portion of the deck to each player
    // Note, if the length is greater than the length of the deck, it will return all remaining cards in the deck, even if they're less than what is requested. 
    return this.shuffledDeck.splice(0, len);
  }
}
Player1 = new Player(false, [], []);
Player2 = new Player(false, [], []);
Deck.BuildDeck();
Deck.ShuffleDeck();
Player1.getCards(Deck.DistributeCards(26)); // Get 26 cards from the shuffled deck and set them as the player's hand
for (var i = 0; i < Player1.currentCards.length; i++) {
  console.log(Player1.currentCards[i][0], Player1.currentCards[i][1], Player1.currentCards.indexOf(Player1.currentCards[i]));
}

Note the addition of the Player.getCards method, just to make things a little more clear (it makes the player's "hand" equal to the cards drawn and passed in), as well as the changes to the Shuffle and Distribute methods.

I tried to make it clear why I changed what I changed, to try and illustrate your problem. Feel free to ask if you need more clarification.

Array.splice documentation

Dirty Developer · Answer 5 · 2017-02-18T03:09:01.997

I have rewritten the logic for shuffling as below:

ShuffleDeck: function() {
    var i = this.deck.length;
    var j = 0;

    while (i--) {
      j = Math.floor(Math.random() * (i + 1));
      this.shuffledDeck.push(this.deck[j]);
      this.deck.splice(j, 1);
    }

    return this.shuffledDeck;   }

working fidlle:https://jsfiddle.net/db6xmnrn/

UPDATE fiddle for delivering cards to player2 from the remaining deck https://jsfiddle.net/db6xmnrn/1/

Array occasionally returning duplicate values

5 Answers5