I am having difficulty with my deal method

Question

In my PokerDeck class I cannot get my deal method to run without producing duplicates. Can anyone tell me what I am doing incorrectly?

import java.util.*;

public class PokerDeck {
    public static final int NUMBER_OF_CARDS = PokerCard.NUMBER_OF_SUITS * PokerCard.NUMBER_OF_RANKS;
    // Instance Variables
    private boolean[] deck;
    private int numberOfCardsInDeck;

    // Constructor
    public PokerDeck()

    {
        deck = new boolean[NUMBER_OF_CARDS];
        for (int j = 0; j < deck.length; j++)
            for (PokerCard.CardSuit suit : PokerCard.CardSuit.values())
                for (PokerCard.CardRank rank : PokerCard.CardRank.values())
                    deck[j] = true;

    }

    // Accessor
    public int getNumberOfCardsInDeck() {
        numberOfCardsInDeck = NUMBER_OF_CARDS;
        return this.numberOfCardsInDeck;

    }

    // Mutator:
    // Return all 52 PokerCards to this PokerDeck
    public void shuffle() {
        for (int i = 0; i < deck.length; i++) {
            int index = (int) (Math.random() * deck.length);
            deck[i] = deck[index];
        }

    }

    // Mutator:
    // Return a randomly selected PokerCard from this PokerDeck
    // Update the state of this PokerDeck to reflect removal of
    // the selected PokerCard
    // Exception thrown if this PokerDeck is "empty"

    public PokerCard deal() {
        Random dealer = new Random();
        int ran = dealer.nextInt(deck.length);
        if (numberOfCardsInDeck == 0)
            throw new RuntimeException("Empty Deck");

        if (deck[ran] == false)
            ran = dealer.nextInt(deck.length);

        deck[ran] = false;
        int suit = ran / 13;
        int rank = ran % 13;

        return new PokerCard(PokerCard.CardSuit.values()[suit], PokerCard.CardRank.values()[rank]);
    }
}

I tried at least five different code and all of them either do not run or they run and return duplicates.

well you aren't marking the PokerCard as dealt before returning, so in the context of your code there can be duplicates. — Orin, Sep 14 '16 at 17:45
the line if (deck[dealer.nextInt(deck.length)] = false) should be changed to: if (deck[dealer.nextInt(deck.length)] == false) , but I'm not sure if that will solve your problem... — Ted, Sep 14 '16 at 17:47
your shuffle is also problematic: you'll lose about half your cards at each shuffle — Adalcar, Sep 14 '16 at 17:50

score 0 · Accepted Answer · edited May 23 '17 at 10:33

0

There is a lot of things wrong with this code. Your problem is, because you are randoming more and more instead of using te ran variable you declared at the beggining of deal method. After you declared your ran variable, don't do dealer.nextInt(deck.length), replace it with ran.

Your shuffling is not really shuffling, check this: Random shuffling of an array.

Comparing (like if(something == false)) should be done with ==, you are doing it with = (but you probably just pasted it wrong).

In your getter this numberOfCardsInDeck = NUMBER_OF_CARDS; is wrong, unless you wanted to 'refresh' a deck instead of just get a number of cards, but I don't think so, remove that line.

Triple for in your constructor is wrong, you need only the first one, it doesn't make a difference here (except performance), but fix it.

Also I suggest rewriting all this, do a Card object (class) with suit and rank enums, make a Deck object with List of Card objects, make a deck.getCard() method that removes the last card from the deck, make the deck.shuffle() method that shuffles deck (you will have a List of Cards now, not an array, so you will be able to do Collections.shuffle(cardList), simple).

Also, change this:

if (deck[dealer.nextInt(deck.length)] == false)
            dealer.nextInt(deck.length);

To something like this:

while (deck[ran] == false)
            ran = dealer.nextInt(deck.length);

edited May 23 '17 at 10:33

Community

1
1

answered Sep 14 '16 at 17:47

Shadov

5,421
2
19
38

my professor says that there is no need to exchange cards and that the shuffle method is supposed to basically do the same as the constructor – Serge Metellus Sep 14 '16 at 17:57
what am i supposed to replace the code with the getter method then? – Serge Metellus Sep 14 '16 at 17:58
my professor provided us with a PokerCard class and a test class so the code has to be with suit and rank enums and a deck arrray of booleans – Serge Metellus Sep 14 '16 at 18:02
Yes, shuffle is wrong, check link I gave you. Remove first line in getter method and put it in constructor. Change that 'if' to 'while' like I wrote in my answer. Add 'numberOfCardsInDeck--' before returning from deal method. – Shadov Sep 14 '16 at 18:05
the link you gave me is saying to swap or exchange the cards which i am not supposed to do. the shuffle method is supposed to basically do the same as the constructor – Serge Metellus Sep 14 '16 at 18:57

score 0 · Answer 2 · answered Sep 14 '16 at 18:07

there are a few scary errors which make me wonder just how much you read your own code:

the reuse of random here:
```
if (deck[dealer.nextInt(deck.length)] == false)
```
means that you will randomize again and not use the previously generated ran.
this block
```
if (deck[dealer.nextInt(deck.length)] == false)
    dealer.nextInt(deck.length);
```
is useless since dealer.nextInt(deck.length) simply creates a value without storing anything. I supposed you were trying to "keep trying until you find a valid card" so it should look more like this:
```
while (deck[ran] == false)
    ran = dealer.nextInt(deck.length);
```
the Shuffle() method here simply duplicates values eveywhere in your deck since instead of swapping you simply replace the first value with the second, losing the first. to avoid that simply add
```
boolean b = deck[i];
deck[i] = deck[index];
deck[index] = b;
```
to make it a swap.

that's all i found after a first reading.

like i said in my previous comment i already spoke to my professor about my shuffle method and he said its supposed to basically do the same as the constructor — Serge Metellus, Sep 14 '16 at 18:55

I am having difficulty with my deal method

2 Answers2