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
– Dirty Developer, Commented Feb 18, 2017 at 3:07

ibrahim mahrir · Accepted Answer · 2017-02-18 02:25:40Z

3

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);

edited Feb 18, 2017 at 2:25

answered Feb 18, 2017 at 0:02

ibrahim mahrir

31.8k5 gold badges50 silver badges78 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Robert Over a year ago

Thank you for explaining everything. Now the issue is what to do when there are two players.

ibrahim mahrir Over a year ago

@Robert check the Edit section!

Community · Accepted Answer · 2017-05-23 12:32:02Z

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, 2017 at 12:32

CommunityBot

11 silver badge

answered Feb 17, 2017 at 23:59

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

15.6k7 gold badges37 silver badges40 bronze badges

3 Comments

Robert Over a year ago

Would shuffling the cards once they're passed into each player's deck fix it?

Brandon Anzaldi Over a year ago

@Robert Probably not, since the distribution would not be random.

Tha'er AlAjlouni ثائر العجلوني Over a year ago

@Robert I totally agree with Brandon.

iWillBeMaster · Accepted Answer · 2017-02-18 00:03:36Z

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, 2017 at 0:03

iWillBeMaster

1571 silver badge11 bronze badges

3 Comments

bormat Over a year ago

why do you use 0.5 ?

ibrahim mahrir Over a year ago

@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!

Robert Over a year ago

This always returns "undefined"

Brandon Anzaldi · Accepted Answer · 2017-02-18 00:26:01Z

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 · Accepted Answer · 2017-02-18 03:09:01Z

0

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/

edited Feb 18, 2017 at 3:09

answered Feb 18, 2017 at 2:35

Dirty Developer

5726 silver badges24 bronze badges

Collectives™ on Stack Overflow

Array occasionally returning duplicate values

5 Answers 5

2 Comments

3 Comments

3 Comments

Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

5 Answers 5

2 Comments

3 Comments

3 Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related