22
\$\begingroup\$

I have been a programmer for 12 years, mainly ERP software and C development and am looking to make a career/specialty change to Java. I've read it countless times if you want to learn a new language you need to write code, then write some more code and then finally write more code. So I've written some code!

I love Poker, so I have written a small Texas Hold'em program. Here is the overview of what it does:

  1. Asks the user for the number of players
  2. Create a deck of cards
  3. Shuffle
  4. Cut the deck
  5. Deal players hole cards
  6. Burns a card
  7. Deals flop
  8. Burns a card
  9. Deals turn
  10. Burns a card
  11. Deals river
  12. Prints the deck to console to show random deck was used
  13. Prints the 'board'
  14. Prints burn cards
  15. Printers players cards
  16. Evaluates the value of each players hand (Royal flush, full house, etc...)

There are 6 .java files (see below for links). I used an interface, created my own comparators, even implemented some try/catch blocks (although I'm still learning how to use these properly).

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;

public class TexasHoldEm {

    public static void main(String[] args) throws Exception {
        // variables
        Deck holdemDeck = new Deck();
        int numPlayers = 0;
        int cardCounter = 0;
        int burnCounter = 0;
        int boardCounter = 0;
        Board board = new Board();

        // initializations      
        numPlayers = getNumberOfPlayers();
        Player[] player = new Player[numPlayers];

        /* 3 shuffles just like in real life. */
        for(int i=0;i<3;i++){
            holdemDeck.shuffle();
        }

        // Cut Deck
        holdemDeck.cutDeck();

        // Initialize players
        for (int i=0;i<numPlayers;i++){
            player[i] = new Player();
        }

        // Main processing
        // Deal hole cards to players
        for (int i=0;i<2;i++){
            for (int j=0;j<numPlayers;j++){
                player[j].setCard(holdemDeck.getCard(cardCounter++), i);
            }
        }

        // Start dealing board

        // Burn one card before flop
        board.setBurnCard(holdemDeck.getCard(cardCounter++), burnCounter++);

        // deal flop
        for (int i=0; i<3;i++){
            board.setBoardCard(holdemDeck.getCard(cardCounter++), boardCounter++);
        }

        // Burn one card before turn
        board.setBurnCard(holdemDeck.getCard(cardCounter++), burnCounter++);

        // deal turn
        board.setBoardCard(holdemDeck.getCard(cardCounter++), boardCounter++);

        // Burn one card before river
        board.setBurnCard(holdemDeck.getCard(cardCounter++), burnCounter++);

        // deal river
        board.setBoardCard(holdemDeck.getCard(cardCounter++), boardCounter++);

        //------------------------
        // end dealing board
        //------------------------

        System.out.println("The hand is complete...\n");

        // print deck
        holdemDeck.printDeck();

        //print board
        board.printBoard();

        // print player cards
        System.out.println("The player cards are the following:\n");
        for (int i=0;i<numPlayers;i++){
            player[i].printPlayerCards(i);
        }

        // print burn cards
        board.printBurnCards();

        //------------------------
        // Begin hand comparison
        //------------------------
        for (int i=0;i<numPlayers;i++){
            HandEval handToEval = new HandEval();

            // populate with player cards           
            for (int j=0;j<player[i].holeCardsSize();j++){
                handToEval.addCard(player[i].getCard(j),j);
            }

            //populate with board cards
            for (int j=player[i].holeCardsSize();j<(player[i].holeCardsSize()+board.boardSize());j++){
                handToEval.addCard(board.getBoardCard(j-player[i].holeCardsSize()),j);
            }

            System.out.println("Player " + (i+1) + " hand value: " + handToEval.evaluateHand());    
        }
    }

    protected static int getNumberOfPlayers() throws Exception{
        int intPlayers = 0;
        String userInput = "";

        // Get number of players from user.
        System.out.println("Enter number of players (1-9):");
        BufferedReader br = new BufferedReader(new InputStreamReader(System.in));

        try { 
               userInput = br.readLine(); 
            } catch (IOException ioe) { 
               System.out.println("Error: IO error trying to read input!"); 
               System.exit(1); 
            }

        // convert user input to an integer
        try {
           intPlayers = Integer.parseInt(userInput);
            } catch (NumberFormatException nfe) {
               System.out.println("Error: Input provided is not a valid Integer!"); 
               System.exit(1); 
            }

        if ((intPlayers<1) || (intPlayers>9)){
            throw new Exception("Error: Number of players must be an integer between 1 and 9");
        }       

        return intPlayers;
    }
}

Player.java

public class Player {
    private Card[] holeCards = new Card[2];

    //constructor
    public Player(){
    }

    public Player(Card card1, Card card2){
        holeCards[0] = card1;
        holeCards[1] = card2;
    }

    //methods
    protected void setCard(Card card, int cardNum){
        holeCards[cardNum] = card;
    }

    protected Card getCard(int cardNum){
        return holeCards[cardNum];
    }

    protected int holeCardsSize(){
        return holeCards.length;
    }

    protected void printPlayerCards(int playerNumber){
        System.out.println("Player " + (playerNumber+1) + " hole cards:");
        for (int i=0;i<2;i++){
            System.out.println(holeCards[i].printCard());
        }
        System.out.println("\n");
    }
}

HandEval.java

import java.util.Arrays;

public class HandEval { 
    private Card[] availableCards = new Card[7];

    private final static short ONE = 1;
    private final static short TWO = 2;
    private final static short THREE = 3;
    private final static short FOUR = 4;

    // Constructor
    public HandEval(){
    }

    //methods
    protected void addCard(Card card, int i){
        availableCards[i] = card;
    }

    protected Card getCard(int i){
        return availableCards[i];
    }

    protected int numCards(){
        return availableCards.length;
    }

    protected void sortByRank(){
        Arrays.sort(availableCards, new rankComparator());
    }

    protected void sortBySuit(){
        Arrays.sort(availableCards, new suitComparator());
    }

    protected void sortBySuitThenRank(){
        Arrays.sort(availableCards, new suitComparator());
        Arrays.sort(availableCards, new rankComparator());
    }

    protected void sortByRankThenSuit(){
        Arrays.sort(availableCards, new rankComparator());
        Arrays.sort(availableCards, new suitComparator());
    }

    protected String evaluateHand(){
        String handResult = new String();
        short[] rankCounter = new short[13];
        short[] suitCounter = new short[4];

        // initializations
        for (int i=0;i<rankCounter.length;i++){
            rankCounter[i] =0;
        }

        for (int i=4;i<suitCounter.length;i++){
            suitCounter[i] = 0;
        }

        // Loop through sorted cards and total ranks
        for(int i=0; i<availableCards.length;i++){
            rankCounter[ availableCards[i].getRank() ]++;
            suitCounter[ availableCards[i].getSuit() ]++;
        }

        //sort cards for evaluation
        this.sortByRankThenSuit();

        // hands are already sorted by rank and suit for royal and straight flush checks.       
        // check for royal flush
        handResult = evaluateRoyal(rankCounter, suitCounter);

        // check for straight flush
        if (handResult == null || handResult.length() == 0){
            handResult = evaluateStraightFlush(rankCounter, suitCounter);
        }

        // check for four of a kind
        if (handResult == null || handResult.length() == 0){
            handResult = evaluateFourOfAKind(rankCounter);
        }

        // check for full house
        if (handResult == null || handResult.length() == 0){
            handResult = evaluateFullHouse(rankCounter);
        }

        // check for flush
        if (handResult == null || handResult.length() == 0){
            handResult = evaluateFlush(rankCounter, suitCounter);
        }

        // check for straight
        if (handResult == null || handResult.length() == 0){
            // re-sort by rank, up to this point we had sorted by rank and suit
            // but a straight is suit independent.
            this.sortByRank();
            handResult = evaluateStraight(rankCounter);
        }

        // check for three of a kind
        if (handResult == null || handResult.length() == 0){
            handResult = evaluateThreeOfAKind(rankCounter);
        }

        // check for two pair
        if (handResult == null || handResult.length() == 0){
            handResult = evaluateTwoPair(rankCounter);
        }

        // check for one pair
        if (handResult == null || handResult.length() == 0){
            handResult = evaluateOnePair(rankCounter);
        }

        // check for highCard
        if (handResult == null || handResult.length() == 0){
            handResult = evaluateHighCard(rankCounter);
        }


        return handResult;
    }

    private String evaluateRoyal(short[] rankCounter, short[] suitCounter){
        String result = "";

        // Check for Royal Flush (10 - Ace of the same suit).
        // check if there are 5 of one suit, if not royal is impossible     
        if ((rankCounter[9] >= 1 &&       /* 10 */
                rankCounter[10] >= 1 &&   /* Jack */
                rankCounter[11] >= 1 &&  /* Queen */
                rankCounter[12] >= 1 &&  /* King */
                rankCounter[0] >= 1)    /* Ace */
                && (suitCounter[0] > 4 || suitCounter[1] > 4 ||
                        suitCounter[2] > 4 || suitCounter[3] > 4)){

            // min. requirements for a royal flush have been met, 
            // now loop through records for an ace and check subsequent cards. 
            // Loop through the aces first since they are the first card to 
            // appear in the sorted array of 7 cards. 
            royalSearch:
                for (int i=0;i<3;i++){
                    // Check if first card is the ace.
                    // Ace must be in position 0, 1 or 2
                    if (availableCards[i].getRank() == 0){
                        // because the ace could be the first card in the array
                        // but the remaining 4 cards could start at position 1,
                        // 2 or 3 loop through checking each possibility.
                        for (int j=1;j<4-i;j++){
                            if ((availableCards[i+j].getRank() == 9 && 
                                    availableCards[i+j+1].getRank() == 10 &&
                                    availableCards[i+j+2].getRank() == 11 &&
                                    availableCards[i+j+3].getRank() == 12) 
                                    &&
                                    (availableCards[i].getSuit() == availableCards[i+j].getSuit() &&
                                    availableCards[i].getSuit() == availableCards[i+j+1].getSuit() &&
                                    availableCards[i].getSuit() == availableCards[i+j+2].getSuit() &&
                                    availableCards[i].getSuit() == availableCards[i+j+3].getSuit())){
                                        // Found royal flush, break and return.
                                        result = "Royal Flush!! Suit: " + Card.suitAsString(availableCards[i].getSuit());
                                        break royalSearch;              
                            }
                        }
                    }               
                }
        }       
        return result;
    }

    // Straight flush is 5 consecutive cards of the same suit.
    private String evaluateStraightFlush(short[] rankCounter, short[] suitCounter){
        String result = "";

        if (suitCounter[0] > 4 || suitCounter[1] > 4 ||
                suitCounter[2] > 4 || suitCounter[3] > 4){

            // min. requirements for a straight flush have been met.
            // Loop through available cards looking for 5 consecutive cards of the same suit,
            // start in reverse to get the highest value straight flush
            for (int i=availableCards.length-1;i>3;i--){
                if ((availableCards[i].getRank()-ONE == availableCards[i-ONE].getRank() && 
                        availableCards[i].getRank()-TWO == availableCards[i-TWO].getRank() &&
                        availableCards[i].getRank()-THREE == availableCards[i-THREE].getRank() &&
                        availableCards[i].getRank()-FOUR == availableCards[i-FOUR].getRank()) 
                        &&
                        (availableCards[i].getSuit() == availableCards[i-ONE].getSuit() &&
                        availableCards[i].getSuit() == availableCards[i-TWO].getSuit() &&
                        availableCards[i].getSuit() == availableCards[i-THREE].getSuit() &&
                        availableCards[i].getSuit() == availableCards[i-FOUR].getSuit())){
                            // Found royal flush, break and return.
                            result = "Straight Flush!! " + Card.rankAsString(availableCards[i].getRank()) + " high of " + Card.suitAsString(availableCards[i].getSuit());
                            break;
                }
            }
        }
        return result;
    }

    // Four of a kind is 4 cards with the same rank: 2-2-2-2, 3-3-3-3, etc...
    private String evaluateFourOfAKind(short[] rankCounter){
        String result = "";

        for (int i=0;i<rankCounter.length;i++){
            if (rankCounter[i] == FOUR){
                result = "Four of a Kind, " + Card.rankAsString(i) +"'s";
                break;
            }
        }   
        return result;
    }

    // Full house is having 3 of a kind of one rank, and two of a kind of 
    // a second rank.  EX: J-J-J-3-3
    private String evaluateFullHouse(short[] rankCounter){
        String result = "";
        short threeOfKindRank = -1;
        short twoOfKindRank = -1;

        for (int i=rankCounter.length;i>0;i--){
            if ((threeOfKindRank < (short)0) || (twoOfKindRank < (short)0)){
                if ((rankCounter[i-ONE]) > 2){
                    threeOfKindRank = (short) (i-ONE);                  
                }
                else if ((rankCounter[i-ONE]) > 1){
                    twoOfKindRank = (short)(i-ONE);
                }
            }
            else
            {
                break;
            }
        }

        if ((threeOfKindRank >= (short)0) && (twoOfKindRank >= (short)0)){
            result = "Full House: " + Card.rankAsString(threeOfKindRank) + "'s full of " + Card.rankAsString(twoOfKindRank) + "'s";
        }

        return result;
    }

    // Flush is 5 cards of the same suit.
    private String evaluateFlush(short[] rankCounter, short[] suitCounter){
        String result = "";

        // verify at least 1 suit has 5 cards or more.
        if (suitCounter[0] > 4 || suitCounter[1] > 4 ||
                suitCounter[2] > 4 || suitCounter[3] > 4){

            for (int i=availableCards.length-1;i>3;i--){
                if (availableCards[i].getSuit() == availableCards[i-ONE].getSuit() &&
                        availableCards[i].getSuit() == availableCards[i-TWO].getSuit() &&
                        availableCards[i].getSuit() == availableCards[i-THREE].getSuit() &&
                        availableCards[i].getSuit() == availableCards[i-FOUR].getSuit()){
                            // Found royal flush, break and return.
                            result = "Flush!! " + Card.rankAsString(availableCards[i].getRank()) + " high of " + Card.suitAsString(availableCards[i].getSuit());
                            break;
                }
            }           
        }


        return result;
    }

    // Straight is 5 consecutive cards, regardless of suit.
    private String evaluateStraight(short[] rankCounter){
        String result = "";

        // loop through rank array to check for 5 consecutive
        // index with a value greater than zero
        for (int i=rankCounter.length;i>4;i--){
            if ((rankCounter[i-1] > 0) &&
                    (rankCounter[i-2] > 0) &&
                    (rankCounter[i-3] > 0) &&
                    (rankCounter[i-4] > 0) &&
                    (rankCounter[i-5] > 0)){
                        result = "Straight " + Card.rankAsString(i-1) + " high";
                        break;
            }
        }
        return result;
    }

    // Three of a kind is 3 cards of the same rank.
    private String evaluateThreeOfAKind(short[] rankCounter){
        String result = "";

        // loop through rank array to check for 5 consecutive
        // index with a value greater than zero
        for (int i=rankCounter.length;i>0;i--){
            if (rankCounter[i-1] > 2){
                        result = "Three of a Kind " + Card.rankAsString(i-1) + "'s";
                        break;
            }
        }
        return result;
    }

    // Two pair is having 2 cards of the same rank, and two
    // different cards of the same rank.  EX: 3-3-7-7-A
    private String evaluateTwoPair(short[] rankCounter){
        String result = "";
        short firstPairRank = -1;
        short secondPairRank = -1;

        for (int i=rankCounter.length;i>0;i--){
            if ((firstPairRank < (short)0) || (secondPairRank < (short)0)){             
                if (((rankCounter[i-ONE]) > 1) && (firstPairRank < (short)0)){
                    firstPairRank = (short) (i-ONE);                    
                }
                else if ((rankCounter[i-ONE]) > 1){
                    secondPairRank = (short)(i-ONE);
                }
            }
            else
            {
                // two pair found, break loop.
                break;
            }
        }

        // populate output
        if ((firstPairRank >= (short)0) && (secondPairRank >= (short)0)){
            if (secondPairRank == (short)0){
                // Aces serve as top rank but are at the bottom of the rank array
                // swap places so aces show first as highest pair
                result = "Two Pair: " + Card.rankAsString(secondPairRank) + "'s and " + Card.rankAsString(firstPairRank) + "'s";
            }
            else
            {
                result = "Two Pair: " + Card.rankAsString(firstPairRank) + "'s and " + Card.rankAsString(secondPairRank) + "'s";
            }           
        }

        return result;
    }

    // One is is two cards of the same rank.
    private String evaluateOnePair(short[] rankCounter){
        String result = "";

        for (int i=rankCounter.length;i>0;i--){
            if((rankCounter[i-ONE]) > 1){
                result = "One Pair: " + Card.rankAsString(i-ONE) + "'s";    
                break;
            }
        }
        return result;
    }

    // high card is the highest card out of the 7 possible cards to be used.
    private String evaluateHighCard(short[] rankCounter){
        String result = "";

        for (int i=rankCounter.length;i>0;i--){
            if((rankCounter[i-ONE]) > 0){
                result = "High Card: " + Card.rankAsString(i-ONE);
                break;
            }
        }
        return result;
    }

}

Deck.java

import java.util.Random;

public class Deck{
    private Card[] cards = new Card[52];

    //Constructor
    public Deck(){
        int i = 0;
        for (short j=0; j<4; j++){
            for (short k=0; k<13;k++){
                cards[i++] = new Card(k, j);    
            }
        }
    }

    // Print entire deck in order
    protected void printDeck(){
        for(int i=0; i<cards.length;i++){
            System.out.println(i+1 + ": " + cards[i].printCard());
        }
        System.out.println("\n");
    }

    // Find card in deck in a linear fashion
    // Use this method if deck is shuffled/random
    protected int findCard(Card card){
        for (int i=0;i<52;i++){
            if (Card.sameCard(cards[i], card)){
                return i;
            }
        }
        return -1;
    }

    //return specified card from deck
    protected Card getCard(int cardNum){
        return cards[cardNum];
    }

    protected void shuffle(){
        int length = cards.length;
        Random random = new Random();
        //random.nextInt();
        for (int i=0;i<length;i++){
            int change = i + random.nextInt(length-i);
            swapCards(i, change);
        }
    }

    protected void cutDeck(){
        Deck tempDeck = new Deck();
        Random random = new Random();
        int cutNum = random.nextInt(52);
        for (int i=0;i<cutNum;i++){
            tempDeck.cards[i] = this.cards[52-cutNum+i];            
        }
        for (int j=0;j<52-cutNum;j++){
            tempDeck.cards[j+cutNum] = this.cards[j];           
        }
        this.cards = tempDeck.cards;
    }

    // Swap cards in array to 'shuffle' the deck.
    private void swapCards(int i, int change){      
        Card temp = cards[i];
        cards[i] = cards[change];
        cards[change] = temp;
    }
}

Card.java

import java.util.*;

public class Card{
    private short rank, suit;

    private static String[] ranks = {"Ace", "2", "3", "4", "5", "6", "7", "8", "9", "10", "Jack", "Queen", "King"};
    private static String[] suits = {"Diamonds", "Clubs", "Hearts", "Spades"};

    //Constructor
    public Card(short rank, short suit){
        this.rank = rank;
        this.suit = suit;
    }

    // Getter and Setters
    public short getSuit(){
        return suit;
    }

    public short getRank(){
        return rank;
    }

    protected void setSuit(short suit){
        this.suit = suit;
    }

    protected void setRank(short rank){
        this.rank = rank;
    }

    // methods
    public static String rankAsString(int __rank){
        return ranks[__rank];
    }

    public static String suitAsString(int __suit){
        return suits[__suit];
    }

    public @Override String toString(){
        return rank + " of " + suit;
    }

    // Print card to string
    protected String printCard(){
        return ranks[rank] + " of " + suits[suit];
    }

    // Determine if two cards are the same (Ace of Diamonds == Ace of Diamonds)
    public static boolean sameCard(Card card1, Card card2){
        return (card1.rank == card2.rank && card1.suit == card2.suit);
    }   

}

class rankComparator implements Comparator<Object>{
    public int compare(Object card1, Object card2) throws ClassCastException{
        // verify two Card objects are passed in
        if (!((card1 instanceof Card) && (card2 instanceof Card))){
            throw new ClassCastException("A Card object was expeected.  Parameter 1 class: " + card1.getClass() 
                    + " Parameter 2 class: " + card2.getClass());
        }

        short rank1 = ((Card)card1).getRank();
        short rank2 = ((Card)card2).getRank();

        return rank1 - rank2;
    }
}

class suitComparator implements Comparator<Object>{
    public int compare(Object card1, Object card2) throws ClassCastException{
        // verify two Card objects are passed in
        if (!((card1 instanceof Card) && (card2 instanceof Card))){
            throw new ClassCastException("A Card object was expeected.  Parameter 1 class: " + card1.getClass() 
                    + " Parameter 2 class: " + card2.getClass());
        }

        short suit1 = ((Card)card1).getSuit();
        short suit2 = ((Card)card2).getSuit();

        return suit1 - suit2;
    }
}

Board.java

public class Board {
    private Card[] board = new Card[5];
    private Card[] burnCards = new Card[3];

    //constructor
    public Board(){
    }

    //methods
    protected void setBoardCard(Card card, int cardNum){
        this.board[cardNum] = card;
    }

    protected Card getBoardCard(int cardNum){
        return this.board[cardNum];
    }

    protected void setBurnCard(Card card, int cardNum){
        this.burnCards[cardNum] = card;
    }

    protected Card getBurnCard(int cardNum){
        return this.burnCards[cardNum];
    }

    protected int boardSize(){
        return board.length;
    }

    protected void printBoard(){
        System.out.println("The board contains the following cards:");
        for(int i =0; i<board.length;i++){
            System.out.println(i+1 + ": " + getBoardCard(i).printCard());
        }
        System.out.println("\n");
    }

    protected void printBurnCards(){
        System.out.println("The burn cards are:");
        for(int i =0; i<burnCards.length;i++){
            System.out.println(i+1 + ": " + getBurnCard(i).printCard());
        }
        System.out.println("\n");
    }

}
\$\endgroup\$
  • 1
    \$\begingroup\$ Please include the main part of the code that you want reviewed in the post as per the FAQ. Thanks \$\endgroup\$ – Mark Loeser Mar 22 '11 at 13:54
  • \$\begingroup\$ @Mark Loeser: Ok, I really want the whole program looked at, the post will be extremely lengthy so I thought posting the code to pastebin would have been a better route. I'll modify my post. \$\endgroup\$ – user2733 Mar 22 '11 at 14:18
  • 1
    \$\begingroup\$ BTW, I don't want to dissuade you to learn Java, but if your Hold'em example is a typical example for the kind of programming you plan to do, then you may be interested in a functional language. Take for example Scala, which basically is Java with functional features. \$\endgroup\$ – RoToRa Mar 24 '11 at 14:42
  • \$\begingroup\$ @RoToRa: I want to learn java, I just couldn't think of a program to write to learn it. Doing tutorials from the internet gets old quick and didn't allow me to solve problems at the level I was interested in. My past experience is in C so that's probably why I wrote the program the way I did. I also wanted to have some code samples so when I interview I'll have something to show them because I won't have any on-the-job experience in the language yet. \$\endgroup\$ – user2733 Mar 24 '11 at 16:22
  • \$\begingroup\$ Poker is notoriously difficult to use with OOP. Blackjack works better with it. Also this tutorial site has a number of exercises for Java that are around the level of Poker as well as exercises that are the building blocks of the more complex exercises. You will also learn GUIs, IO, Threads, ADTs, and Generics, which are IMHO at a higher level than Poker. I think tis pretty good stuff :) \$\endgroup\$ – Eva Jan 16 '13 at 11:41
38
\$\begingroup\$

Let's start with the most basic, the Card class.

import java.util.*;

Except during development, it's custom to explicitly import only the classes you need instead of using wildcards.

public class Card{
    private short rank, suit;

It's is most certainly a valid choice to store rank and suit as shorts, since it's most likely the fastest and most efficient way, but if you want to learn the specifics of Java you may want to look into enumerations.

protected void setSuit(short suit){
    this.suit = suit;
}

protected void setRank(short rank){
    this.rank = rank;
}

IMHO cards are a prime example for immutable objects. There is no reason to need to change the rank or the suit of a card, so I would drop the setters.

public static String rankAsString(int __rank){
    return ranks[__rank];
}

public static String suitAsString(int __suit){
    return suits[__suit];
}

It's very unusual to use underlines in variable names in Java, especially two as a prefix. I would simple name the parameters rank and suit especially since these are class (static) methods and not instance methods, so there is no confusion with the fields.

It may be worth thinking about, if these actually need to be class methods and shouldn't be instance methods. If you have other classes which need to convert short into the corresponding names independently from the Card class, then it would be ok. But in your case I would say it's not the case, and one should try and hide the fact that suits and ranks are implemented as shorts as much as possible.

public @Override String toString(){
    return rank + " of " + suit;
}

// Print card to string
protected String printCard(){
    return ranks[rank] + " of " + suits[suit];
}

The Java community is split over the question, if the toString() method should be overridden purely for debugging reasons, or if it should be used in the "business logic". In this "simple" application I don't think you need to distinguish between the two uses, so I would drop printCard() and only use toString().

BTW, it's custom to have annotations before the method modifiers.

public static boolean sameCard(Card card1, Card card2){
    return (card1.rank == card2.rank && card1.suit == card2.suit);
}

Instead of implementing your own method it's probably a good idea to override equals() (or at least make this an instance method). If you make Card immutable as I suggested before, it simplifies the implementation to comparing the references, to see if they are the same, because you should only have one instance of each possible card.

@Override public boolean equals(Object that) {
  return this == that;
}

(Although it may be safer to compare rank and suit as a fall back.)


EDIT 1:

First a quick digression about Enumerations. The Enums have an ordinal number and a compareTo method, allowing you to sort them. You also can assign them properties and create your own order based on those.

The offical language guide has examples for suit and rank enumations and for extending enumerations with your own properties using planets as an example: http://download.oracle.com/javase/1.5.0/docs/guide/language/enums.html

When/if I get to the hand ranking (I haven't looked at it yet) I may be able to give some suggestions have to implement it with enums.


Next are the Comparators. I don't have much experience with them, so I can only give some general suggestions:

  • Classes should always start with a capital letter in Java.
  • You should extend them from Comparator<Card> instead of Comparator<Object>, since you only need to compare cards with each other and not with any other objects.
  • While it is good extra practice, you may want to skip the suit comparator, because it's not really needed in Poker in general (and Texas Hold'em specifically). Suits never have an order in hand ranking usually only needed in some "meta" contexts (such as randomly determinating the position of the button), that currently don't apply to your program. However if you do keep it, then you should correct the order of the suits, because the official ranking is (from lowest to highest) "Clubs", "Diamonds", "Hearts" and "Spades".

Next up, is the Board. First off, I'd like to say, that I'm nor sure if I'd use a Board class like this in a real world Poker application. However off the top of my head I can't think of a different way to do it and for practice this is perfectly fine.

The only thing I would change is instead of explicitly setting each card by index with setBoardCard(Card card, int cardNum), I would let the Board class track the index internally itself and just use addBoardCard(Card card), because you shouldn't be able to "go back" and change the board cards (ditto for the burn cards).


EDIT 2:

Regarding sorting suits: Ok, that makes sense. I haven't looked at hand evaluation yet. However that is more a case of grouping that sorting, so maybe there is a different (better?) way to do that. I'll have to think about it.


Tracking indexes: You certainly could use a Collection (or more specifically a List) to do this (and it would be more "Java-like", but in your case where you have a fixed maximum number of cards on the board the arrays are fine. I'd something like:

public class Board { //abbreviated

    private Card[] board = new Card[5];
    private Card[] burnCards = new Card[3];

    private int boardIndex = 0;
    private int burnCardIndex = 0;

    //methods
    protected void addBoardCard(Card card){
        this.board[boardIndex++] = card;
    }

    protected void addBurnCard(Card card){
        this.burnCards[burnCardIndex++] = card;
    }
}

Next is Deck:

First I'd suggest to create just one Random object statically instead of creating one for each call of shuffle and cutDeck.

Really problematic is the creation of a new temporary Deck object for cutting the deck. This can go wrong very fast, because it contains an unnecessary second set of cards and if there is a bug you'll easily get duplicate cards. Instead just use a new array. Also you can use System.arraycopy to simplify copying from one array to another.

EDIT 3:

Creating a new Deck doesn't only create a new array, it creates a a filled array with new cards, which you don't need. Just an array is enough:

protected void cutDeck(){
    Card[] temp = new Card[52];
    Random random = new Random();
    int cutNum = random.nextInt(52);

    System.arraycopy(this.cards,      0, temp, 52-cutNum,   cutNum);
    System.arraycopy(this.cards, cutNum, temp,         0, 52-cutNum);

    this.cards = temp;
}

EDIT 4:

There is not much to say about Player except, I'd remove setCard and just use the constructor to assign the cards to the player in order to make the object immutable. Or at least implement an addCard method just like in Board.


The main class:

In getNumberOfPlayers your error handling it some what inconsistent. On the one hand you write to System.out (System.errwould probably be better) and on the other hand you throw an exception.

For the IOException I wouldn't catch it here, but outside of getNumberOfPlayers. In bigger projects it may make sense to "wrap" it in your own Exception class for this:

try { 
   userInput = br.readLine(); 
} catch (IOException ioe) { 
   throw new HoldemIOException(ioe);
}

Both the caught NumberFormatException and invalid range should throw the same (or related) custom exceptions. Don't just throw a simple Exception, because it's meaningless to others that need to catch it. Example:

try {
  intPlayers = Integer.parseInt(userInput);
} catch (NumberFormatException nfe) {
  throw new HoldemUserException("Error: Not an integer entered for number of players", nfe);
}

if ((intPlayers<1) || (intPlayers>9)){
  throw new HoldemUserException("Error: Number of players must be an integer between 1 and 9");
}

Notice that in the causing IOException and NumberFormatException, are passed as an argument to the new exception in case they are needed further down the line.

Both the HoldemIOException and HoldemUserException could be extended from a basic HoldemException which in turn extends Exception. A simple "empty" extention such as

class HoldemException extends Exception {}

for all three cases would be enough.

Also you never should let an exception (especially a self thrown one) just drop out at the end completely unhandled. Catch all exceptions you know of at a reasonable place, in your case at the getNumberOfPlayers:

do {
  try {
    numPlayers = getNumberOfPlayers();
  } catch (HoldemUserException e) {
    System.out.println(e.getMessage());
    // "Only" a user error, keep trying
  } catch (HoldemIOException e) {
     System.err.println("Error: IO error trying to read input!"); 
     System.exit(1); 
  }
} while (numPlayers = 0);

I only added the do while loop, to show how to handle the two types of exception differently. It would be wise to add a proper way for the user to get out of the loop.


Dealing the cards:

Here we have another counter (cardCounter) to track to "position" of the deck. Again it would be better to have the Deck class track the dealt cards. You should consider implementing the Deck as an actual "stack" or a "queue" - it doesn't matter which, since you aren't adding any items. Java provides a Queue interface you could use.

Thinking about it, you could also use the same interface for Player and Board (although you'd need to separate the burn cards into its own object in that case). That would simplify dealing to player.add(deck.remove());, board.add(deck.remove()); and burnCards.add(deck.remove()).


EDIT 5:

Ok, most likely the final edit.

I've started looking at the hand evaluation and I don't think I can write much about that. You've implemented it in a very procedural way based on your current Card objects and if your goal is to do this in a more Java way you'd probably need to re-write the Card object and create a proper "Hand" object first (most likely based on a Set) and then re-write the evaluation based on that.

Poker hand evaluation is a very complex topic, especially when considering 5 cards out of 7. How this should be implemented will depend on if you want to focus on "good Java practice" or "speed". If you really interested in expanding this, then you probably should first read up on the topic - there are several questions on Stack Overflow about this, and probably countless articles on the web - and then repost a new question focusing on hand evaluation. I'll be happy to have a look at that some day - if I have the time.

Just one specific thing: What are the constants ONE, TWO, THREE and FOUR for? Even for a procedural approach these seem to be completely out of place and should most likely be replaced by an appropriate loop where they are used.


Finally, have fun and good luck on your further adventures in Java land!

\$\endgroup\$
  • \$\begingroup\$ @RoToRa: Thanks, I'll try to post my responses as individual comments. RE: import I changed the java.util.* to java.util.Comparator \$\endgroup\$ – user2733 Mar 22 '11 at 22:00
  • \$\begingroup\$ RE Enumeration: I originally did have the ranks and suits as enums, but when I started to think through how I would need to sort my cards by rank and suit I switched them to short's I didnt know if the enum would cause sorting problems for the HandEval.class, but I knew for certain short's would work and I didnt want to get so far along that it would take a significant amount of refactoring if I had to switch it later. \$\endgroup\$ – user2733 Mar 22 '11 at 22:02
  • \$\begingroup\$ RE: Sorting by suit. I found I needed a Comparator for both suit and rank because of Royal & Straight Flushes. For example if I had 7h-8h-8s-9h-9c-10h-Jh. I needed a way to sort these so I could check for a sequential 7 8 9 10 J of hearts, if I sorted by rank alone the evaluation would fail. One suit doesn't hold a higher precedence over another, just makes evalaution easier. \$\endgroup\$ – user2733 Mar 23 '11 at 13:12
  • \$\begingroup\$ Re Tracking indexes internally - do you have a standard way you do this? Is it will Collection<List>'s? I'd be interested in reading on if it you have a resource or a standard way you do that. Thanks again. \$\endgroup\$ – user2733 Mar 23 '11 at 13:13
  • \$\begingroup\$ Re: Internal indexing, I really like that idea. I'll try to implement that today, and use a unified Random object for both the shuffle and cut deck. I dont understand what you mean by just create another array for the cutDeck, creating a temp deck does create another array to use... ? \$\endgroup\$ – user2733 Mar 24 '11 at 13:58
3
\$\begingroup\$

Consider separating the TexasHoldem class with all its logic (amount of players, counters, ...) from how you initialize it.

Your main would go in a TexasHoldemProgram class, which instantiates a TexasHoldem class, and sets e.g. the amount of players as input in main.

This practice is called the Model View Controller pattern. By separating the logic of the game from the interaction with it, you can later reuse the class with an entirely different GUI. You should probably apply this in other parts of the code as well, but I didn't look thoroughly through it all yet.

The rest of your implementation looks pretty well structured, but I might update this answer if I find other things worth mentioning which aren't mentioned in the other answers. Good luck exploring Java!

\$\endgroup\$
  • \$\begingroup\$ I sort of understand a MVC from a 60,000 ft level, but I'm not sure I understand how I would implement it here. Would I create another class who's sole purpose would be to create a TexasHoldem object? I'm actually very interested if you could explain this, once I get the logic part down I do want to implement a user interface for this program. So any groundwork I can lay now that will make it easier for me later would be appreciated. \$\endgroup\$ – user2733 Mar 23 '11 at 20:19
  • \$\begingroup\$ I notice from your code you do understand how to separate your code. MVC just goes a step further and separates model from the view from the 'controller'. I could write a broad answer here, but others have already done it before me (and better I might add). I suggest you familiarize yourself with design patterns in general, as MVC is a compound pattern. I highly recommend Head First Design Patterns. It's a really entertaining and useful read. If you already know the Strategy and Observer pattern, you can dive into MVC directly. \$\endgroup\$ – Steven Jeuris Mar 23 '11 at 20:45

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy