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:
This way you can have as many players as you want without any weird if-trees.
This is a blatant code smell:
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:
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:
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.
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 drawnCardIt 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.