Coding Help Thread

Without the actual code, it's not really plausible to help you, at least for me. Though one question I will ask is when you say hangs forever, am I to assume it's running for a long awhile(easily multiple million loops)?

Really though, I don't think anyone here who can read your code could not duplicate it on their own, so I think you should post it so we can try to spot a problem, assuming there is one.

Rushed to fix a problem before I forgot it and forgot to post the code.

I think you forget when to set goodNewCard to true.

Code:

public static Card[] drawPokerHand() {
        Card[] pokerHand = new Card[5];
        int[] prevVal = new int[4];
        Random gen = new Random();
        int randNum;
        boolean goodNewCard;
        
        for(int i=0; i<5; i++) {[B]
            goodNewCard = false;
            do {
                randNum = gen.nextInt(52)+1;
            } while(!goodNewCard);[/B]
        }
        
        return pokerHand;
    }

EDiT: dammit, ninja'd

Please pardon the syntax errors. I don't have Netbeans installed that I can change this.

Code:

public static Card[] drawPokerHand() {
        static final int numCards = 5;
        Set pokerValues = new HashSet<Block>();
        Card[] pokerHand = new Card[5];
        Random gen = new Random();
        int randNum;
        
        while(pokerValues.size() + 1 < numCards)
        {
            randNum = gen.nextInt(52)+1;
            pokerValues.add(randNum)
        }
        
        //From here take the values from the HashSet and put it in the pokerHand array.
        
        return pokerHand;
    }

http://download.oracle.com/javase/6/docs...l/Set.html

The other option is to create a "Deck" singleton object, then have the "Poker" class draw from the Deck object to add unique values. Otherwise you could have multiple players that draw the same card twice. So you could have two different straight heart flushes with the same cards.

So, the Deck class would have the following methods:

public GenerateDeck
public ShuffleCards
public DrawCard

And the following properties:

private cards

Sorry about the double post, but this irked me.

When you're looking for a straight, there are a limited number of straights that can exist. And a straight must always be through a line (unless you want a weird straight like Q K A 2 3). So I think it would be far better to do this

PSEUDOCODE:

Code:

hand = GetHand()
numCardsInStraight = 0
cardTypes = [2, 3, 4, 5, 6, 7, 8, 9, 10, J, Q, K, A]
For eachCardType in cardTypes:
   if eachCardType in hand:
       numCardsInStraight += 1
   else:
       numCardsInStraight = 0

   if numCardsInStraight == 5:
      return True
return False

I don't know if this helps you with your problems, but it's a different way to approach it in a much more logical manner than checking next and previous cards in an array.

I would go with Fiel's suggestion of having a deck object and drawing cards from it. That way, generating a card always only takes 1 try, as opposed to your current method which has a worst-case time of infinity (in theory, if the random generator keeps picking the same card over and over). The effect if you're only generating a 5-card hand is small, but as Fiel also mentioned, having a deck object is more realistic as the two players shouldn't both be holding the same card, not to mention better OOP.

Whenever you sense that something is tedious in programming it usually indicates a code smell. And this is definitely a code smell.

The deck class does not deal cards out to players. Have you ever seen a deck of cards, in real life, deal cards to each player? That doesn't make logical sense. The Dealer does this or - in this case - the poker class should do it. The poker class knows how many hands/players there are to deal. A deck of cards does not know nor care how many players/hands there are. It only knows how to let other people shuffle and draw cards.

The poker class should also deal with point values for different cards. Think of it this way. You can also have a deck object for when you play Blackjack, but the same point totals for each card do not apply in Blackjack as they do in poker, right? That's because having Two Jacks in your hand isn't the same across all decks in all games. So the fact that Two Jacks is X amount of points is special to Poker. So it should be in the Poker class.

I've definitely thought of it that way. It just made sense at the time that a card should come out from a deck (a larger unit), which is how I view the relationship for this particular case, and so should a poker hand (5 cards).

It only makes sense though, that a card and a deck are basic building blocks for any card game. A card should have only 2 properties: the value and the name for that value. A deck, I don't know, it probably has more methods than variables. Everything else is Poker-exclusive.

I think if you studied "has a" and "can" relationships you'd be much better off in OOP.

What you're describing here is: "A deck has a poker hand" (FALSE). "A deck can deal cards to players" (FALSE). "A Card has a suit" (TRUE). "A Card has a value" (TRUE). You're not totally off-base here, but some of your programming doesn't make logical sense. I'd recommend you fix it.

Sorry about the double post, but this irked me.

When you're looking for a straight, there are a limited number of straights that can exist. And a straight must always be through a line (unless you want a weird straight like Q K A 2 3). So I think it would be far better to do this

PSEUDOCODE:

Code:

hand = GetHand()
numCardsInStraight = 0
cardTypes = [2, 3, 4, 5, 6, 7, 8, 9, 10, J, Q, K, A]
For eachCardType in cardTypes:
   if eachCardType in hand:
       numCardsInStraight += 1
   else:
       numCardsInStraight = 0

   if numCardsInStraight == 5:
      return True
return False

I don't know if this helps you with your problems, but it's a different way to approach it in a much more logical manner than checking next and previous cards in an array.

I see. It's essentially the same approach I used for determining quads, trips, full houses and other stuff: the unique number of occurrences of a certain target.
So just to be clear on the idea, you will start the "combo" count, for lack of better words, once an eachCardType in the array of length 13 can be found in the hand, then increment it each time that event takes place again, terminate once it is false (so that 2, 3, 4, 5, 8 don't make a straight) and start a new count again, which will never reach the desired number 5.
Am I reading that right?

I'll have to think of a way to deal with A, 2, 3, 4, 5 in that case though. It gives ultimately the same output but I much prefer it since I don't have to change the hand that was supposed to be randomly generated.

I see. It's essentially the same approach I used for determining quads, trips, full houses and other stuff: the unique number of occurrences of a certain target.
So just to be clear on the idea, you will start the "combo" count, for lack of better words, once an eachCardType in the array of length 13 can be found in the hand, then increment it each time that event takes place again, terminate once it is false (so that 2, 3, 4, 5, 8 don't make a straight) and start a new count again, which will never reach the desired number 5.
Am I reading that right?

I'll have to think of a way to deal with A, 2, 3, 4, 5 in that case though. It gives ultimately the same output but I much prefer it since I don't have to change the hand that was supposed to be randomly generated.

Yeah. For example, if your hand is 2 2 3 4 6, logic goes
2: in hand -> numCards += 1 (to 1)
3: in hand -> numCards += 1 (to 2)
4: in hand -> numCards += 1 (to 3)
5: not in hand -> numCards = 0
6: in hand -> numCards += 1 (to 1)
7: not in hand -> numCards = 0
8: not in hand -> numCards = 0
9: not in hand -> numCards = 0
10: not in hand -> numCards = 0
J: not in hand -> numCards = 0
Q: not in hand -> numCards = 0
K: not in hand -> numCards = 0
A: not in hand -> numCards = 0
Since numCards is never 5, it then returns false.


As for A2345, if that's a valid straight, you just make the array {A, 2, 3, ...} and it should work out okay. Doesn't matter that the array duplicates itself on the ends.

You still have some smell, but it's not nearly as bad as before.

The amount of players you can have in a poker game is not limited to 3 (though I can see how you'd want to do that for testing). The best way to figure out who the winner is would be like the following:

Code:

winner = 0
maximumPointTotal = 0
for eachPlayer in Players:
    if eachPlayer.evaluateHand() > maximumPointTotal:
        winner = currentPlayer
        maximumPointTotal = eachPlayer.evaluateHand()
System.out.println("Player " + winner + " won with " + maximumPointTotal + " points");

This way you can have as many players as you want without any weird if-trees.

This is a blatant code smell:

Code:

deck.getCards()[i]

Seek to get rid of it everywhere you can. Does the dealer need to know the location of all of the cards in the deck in order to play Poker? Most assuredly it doesn't. Instead, use "deck.drawCard()" which removes one card from the top of the deck (hint - the deck class should have a property called "currentCardInStack"). Other classes should not know how a deck is represented in the computer (it could be an array. It could be a file. It could be a string). By accessing the deck object as an array you break encapsulation and increase maintenance. Through a mathematical or logical error you could draw a card which doesn't exist (card #53, card #81) or draw duplicate cards. Using a drawCard() method ensures this can never happen.

The poker class should use deck.drawCard() to get the card from the deck, then add the card to the player's hand using player.addCard(). This way a player can have 5 cards, 7 cards, or any number of cards.

For dealing cards, you should do:

Code:

For eachPlayer in Players:
    for eachCard in NUMCARDSINHAND:
         Card drawnCard = new Card();
         drawnCard.setCard(deck.drawCard());
         eachPlayer.giveCard(drawnCard);

So, literally, it looks like the poker dealer is drawing a card for himself, then giving that card to the other player. Then, in the player class within the giveCard method, he puts it in his hand - which is an array of cards.

Your deck class should look like this:

Code:

def drawCard()
    if this->currentCardInDeck > this->sizeDeck;
        raise Exception("Drawn too many cards for 1 deck!");
    Card drawnCard = new Card();
    drawnCard.setCard(this->deck[this->currentCardInDeck]);
    this->currentCardInDeck++;
    return drawnCard

It is safe to know that the deck object returns integers between 0 and 51 to represent cards. What is not safe to know, however, is how the deck generates these numbers or how it's represented internally in the class.

Finally, do not hard code numbers. I notice throughout that you have the magic number "5" to represent 5 cards in your hand. What if you want to change to 7 card stud? Well, then you have to go through the entire program and change all the 5's to 7's. Oh, but now you want regular poker again...? Create a new static final int property in the poker class - "NUMCARDSINHAND" - which is equal to 5. Then use this variable everywhere in the class.

At the end of the round, the poker class runs through every player's hand and determines which hand is the best hand.

OOP in Java should emulate real life as much as possible. By doing this you will eliminate a lot of code smell.