Poker game implementation

Question

I have included almost all features that real poker game contains, but even tho I tried to make it as clean as possible, I think it can be done much better. I used inheritance to somewhat distinct player seating, getting ready for hand to actually playing the game, but then again some things as moving the button depends on seated players. So I don't know what to do here. Some pointers would be great. Also getting state of hand, table seem as a problem, since not every detail about game is accessible from child class.

I want to say that I don't ask for specific implementation, but what should be designed differently to approach my goals much easily and of course improved code quality.

In my opinion there are 3 relevant classes: PokerTable, PokerGame and Player.

I have also created a question some time ago about implementing a Poker Card Ranker.

PokerTable

 namespace Poker.Game
{
public abstract class PokerTable {

    /*
     * Represents first bet size.
     * It is a positive number.
     */ 
    protected readonly Decimal SmallBlind;

    /*
     * Represents second bet size.
     * Is atleast two times bigger than smallBlind.
     * And it is positive number.
     */
    protected readonly Decimal BigBlind;

    /*
     * Maximum amount of players at table.
     */ 
    private readonly uint MaxPlayers;

    /*
     * Contains all players who are in current hand(not folded, not waiting).
     * If player folds/leaves then he is removed from this list.
     */
    protected CircularList<Player> playersInHand;

    /*
     * Contains all players who are sitting at table either waiting or playing a hand.
     * key is seat at which player is seated,
     * value is Player who is sitting there.
     */
    private Dictionary<int, Player> playersAtTable = new Dictionary<int, Player>();

    /*
     * Creates basic game instance with setting blind and max player count.
     */
    public PokerTable(Decimal smallBlind, Decimal bigBlind, uint maxPlayers)
    {
        if (bigBlind / 2 < smallBlind) throw new ArgumentException("Big blind must be twice as big as small blind");
        if (smallBlind <= 0) throw new ArgumentException("Small blind must be positive");
        if (bigBlind <= 0) throw new ArgumentException("Big blind must be positive");

        SmallBlind = smallBlind;
        BigBlind = bigBlind;
        MaxPlayers = maxPlayers;
    }

    /*
     * Seats player at table, it doesn't mean he will be in the hand.
     * He might be in the next hand.
     */ 
    public void addPlayer(int seat, Player player)
    {
        if(seat < 0 || seat >= MaxPlayers) throw new ArgumentException("Player can sit only at [0;" + (MaxPlayers - 1) + "]");

        playersAtTable[seat] = player;
    }

    /*
     * This method starts Game: Deals cards and players
     * can make their bets/folds etc.
     * 
     * If there is less then 2 players the exception will be thrown
     * since one player can't play this game.
     */ 
    public void StartHand()
    {
        if(playersAtTable.Count < 2) throw new LackOfPlayersException();

        //Moves players sitting at table to list of players who are playing a hand
        List<Player> playersAtTable0 = new List<Player>();
        foreach (var entry in playersAtTable)
        {
            playersAtTable0.Add(entry.Value);
        }
        playersInHand = new CircularList<Player>(playersAtTable0);

        StartAction();
    }

    /*
     * This method is called when game can start and playersInHand are set.
     */ 
    protected abstract void StartAction();


    public string GetTableState()
    {
        string state = "";
        foreach (var entry in playersAtTable)
        {
            state += entry.Value.ToString() + "\n";
        }
        return state;
    }
}

public class LackOfPlayersException : Exception { }
}

PokerGame

 namespace Poker.Game
{
public class PokerGame : PokerTable, TableAction
{

    /*
     * Helper var for preflop end of street counter;
     */ 
    private int overlap = 0;

    /*
     * Represent number of cards each player has in his hand.
     */ 
    private const int CARDS_PER_PLAYER = 2;

    /*
     * Represents amount of cards on each street
     */ 
    private const int PRE_FLOP = 0;
    private const int FLOP = 3;
    private const int TURN = 4;
    private const int RIVER = 5;

    /*
     * Represents chips in middle of the table
     */
    private Decimal chipsInPot = 0;

    /*
     * Contains cards to deal to players and to set at middle of the table
     */ 
    private Deck deck;

    /*
     * Contains all cards that are in the middle of table
     */ 
    private List<Card> tableCards = new List<Card>();

    /*
     * Represents player who had the first action in each street
     * (preflop, flop, turn, river). It's used to determine when 
     * round is finished.
     */ 
    private Player firstPlayer;


    public PokerGame() : base(5, 10, 6)
    {

    }

    /*
     * This method is called when there is enough players
     * to start the game and playersInHand are set, but 
     * no blinds are posted.
     */ 
    protected override void StartAction() {
        deck = new Deck();
        deck.Shuffle();
        overlap = 0;

        PostBlinds();
        dealCards();
    }

    private void dealCards()
    {
        foreach (Player player in playersInHand)
        {
            player.Cards = deck.DrawCards(CARDS_PER_PLAYER);
        }
    }

    /*
     * Posts small blind, big blind and moves action to the 
     * next player.
     */ 
    private void PostBlinds()
    {
        playersInHand.getSelected().MakeABet(SmallBlind);
        playersInHand.incrementCounter();

        playersInHand.getSelected().MakeABet(BigBlind);
        playersInHand.incrementCounter();

        firstPlayer = playersInHand.getSelected();
    }

    /*
     * Next player in the line will make a raise to specified amount.
     */
    public void MakeABet(Decimal amount)
    {
        Player prevPlayer = playersInHand.getPrevious();
        amount = (amount < prevPlayer.Bet) ? prevPlayer.Bet : amount;

        playersInHand.getSelected().MakeABet(amount);
        playersInHand.incrementCounter();
    }

    /*
     * Next player in line will make a call.
     * 
     * if everyone is called and everyone has had an action then
     * next card is dealt.
     */
    public void MakeACall()
    {
        //sets player to action starter
        if (firstPlayer == null) firstPlayer = playersInHand.getSelected();

        Player better = playersInHand.getPrevious();
        Decimal callAmount = better.Bet;

        playersInHand.getSelected().MakeABet(callAmount);
        playersInHand.incrementCounter();

        if (IsEndOfStreet())
        {
            DealNextStreet();
            firstPlayer = null;
            if(playersInHand != null) playersInHand.resetCounter();
        }
    }

    /*
     * Checks if it is end of the street
     */ 
    private bool IsEndOfStreet()
    {

        if (playersInHand.getSelected() == firstPlayer || playersInHand.getPrevious().Bet != 0)
        {
            Player prevPlayer = playersInHand.getPrevious();
            Player currPlayer = playersInHand.getSelected();

            if (tableCards.Count == PRE_FLOP && overlap == 0)
            {
                overlap++;
            }

            if (prevPlayer.Bet == currPlayer.Bet)
            {

                if (tableCards.Count == PRE_FLOP && overlap == 0)
                {
                    overlap++;
                } else {    
                    return true;
                }

            }
        }
        return false;
    }

    /*
     * Moves chips to pot, deals next card
     * if it's end of the river then calculate winner 
     * and gives him all pot chip.
     */
    private void DealNextStreet()
    {
        MoveBetsToPot();

        switch (tableCards.Count)
        {
            case PRE_FLOP:
                tableCards.AddRange(deck.DrawCards(FLOP));
                break;
            case FLOP:
                tableCards.AddRange(deck.DrawCards(TURN - FLOP));
                break;
            case TURN:
                tableCards.AddRange(deck.DrawCards(RIVER - TURN));
                break;
            case RIVER:
                EndGame();
                break;
            default:
                throw new ArgumentOutOfRangeException("Not accepted card count " + tableCards.Count);
        }
    }


    /*
     * Move players raises/calls to chopInPot and sets
     * their bets to 0.
     */ 
    private void MoveBetsToPot()
    {
        foreach (Player player in playersInHand)
        {
            chipsInPot += player.Bet;
            player.ResetBet();
        }
    }

    /*
     * Next player in line will fold. Player will be removed from
     * playersInHand list and hes bet will be moved to pot
     */
    public void MakeAFold()
    {
        Player player = playersInHand.getSelected();

        chipsInPot += player.Bet;
        player.Reset();

        playersInHand.DropSelected();


        if (playersInHand.Count == 1)
        {
            MoveBetsToPot();
            EndGame();
        } 
        else if (IsEndOfStreet())
        {
            DealNextStreet();
            firstPlayer = null;
            if (playersInHand != null) playersInHand.resetCounter();
        }
    }


    /*
     * Calcualtes winner and gives him all chips that are in the pot
     * Resets cards from last hand.
     */ 
    private void EndGame()
    {
        List<Player> winners = PokerWinner.GetList(playersInHand, tableCards);

        Decimal wonSize = chipsInPot / winners.Count; //divide by zero exception

        foreach (Player winner in winners)
        {
            winner.AddToStack(wonSize);
        }

        chipsInPot = 0;

        //resets cards, maybe other stuff too..
        foreach (Player player in playersInHand)
        {
            player.Reset();
        }
        playersInHand = null;
    }


    public string GetGameState()
    {
        string info = "Stakes are: " + SmallBlind + "/" + BigBlind + "\n";
        string table = base.GetTableState() + "\n";
        string pot = "Pot is: " + chipsInPot.ToString() + "\n";
        string actionOnPlayer = "Player on action: ";
        actionOnPlayer += (playersInHand != null) ? playersInHand.getSelected().ToString() : "None is on action\n";
        string tableCardsStr = "Table Cards: " + string.Join(",", tableCards) + "\n";
        return info + table + pot + tableCardsStr + actionOnPlayer + "\n";
    }

}

/*
 * This class detects winner or winners from their hand and from the board
 */ 
static class PokerWinner
{
    /*
     * Returns a list of players between whom pot should be split
     */ 
    public static List<Player> GetList(List<Player> players, List<Card> tableCards)
    {
        List<PlayerPokerHand> playerHands = new List<PlayerPokerHand>();

        foreach(Player player in players) {
            PlayerPokerHand playerHand = new PlayerPokerHand();
            List<Card> playerCards = new List<Card>(tableCards);
            playerCards.AddRange(player.Cards);

            playerHand.Hand = PokerRanker.GetPokerHand(playerCards);
            playerHand.Player = player;

            playerHands.Add(playerHand);
        }

        playerHands.Sort();

        for (int i = 1; i < playerHands.Count; i++)
        {
            if (playerHands[0].CompareTo(playerHands[i]) != 0)
            {
                playerHands = playerHands.GetRange(0, i);
                break;
            }
        }

        List<Player> winners = new List<Player>();
        for (int i = 0; i < playerHands.Count; i++)
        {
            winners.Add(playerHands[i].Player);
        }

        return winners;
    }

    /*
     * This is structure to keep location of Players when PokerHands
     * are sorted.
     */ 
    class PlayerPokerHand : IComparable<PlayerPokerHand>
    {
        public Player Player;
        public PokerHand Hand;

        public int CompareTo(PlayerPokerHand other)
        {
            return Hand.CompareTo(other.Hand);
        }

    }
}
}

Player

  namespace Poker.Game
  {
public class Player
{
    private int counter = 0;


    private readonly int id;


    public readonly string Name;

    /*
     * Amount of chips that player has. Not in pot nor raised. 
     */ 
    private decimal stack = 0;

    /*
     * Amount of chips that player has raised or called, but round is not 
     * finished. After finishing round these chips are moved black to player(if he wins) 
     * of moved to the pot if game continous to the next round.
     */ 
    private decimal bet = 0;

    /*
     * Cards that player has during hand. Ussualy it will be in the holdem.
     * If he dont have any then null is set.
     */ 
    private List<Card> cards;

    /*
     * This creates player with name and stack. Stack must be >= 0.
     */ 
    public Player(string name, Decimal stack)
    {
        this.Name = name;
        this.Stack = stack;

       id = Interlocked.Increment(ref counter);
    }

    /*
     * Will throw exception if assigning negative value to stack
     */ 
    public decimal Stack
    {
        get{
            return stack;
        }
        set
        {
            if (value < 0) throw new ArgumentException("Stack can't be negative");

            stack = value;
        }
    }

    /*
     * This method is called when player calls, bets or raised.
     * 
     * betAmount - is amount to make bet equal to or less then
     * That means when player needs to call 100, but already has betted 50
     * then betAmount should  be 100. It will subtrack 50 from stack and add
     * to bet variable.
     * 
     * If player has less then hipotetical 100 chips then maximum amount will be added to the pot.
     */ 
    public void MakeABet(Decimal betAmount)
    {
        if (betAmount < 0) throw new ArgumentException("Bet can't be negative");

        Decimal amount = betAmount - bet;
        betAmount = (Stack < amount) ? Stack : amount;

        bet += betAmount;
        Stack -= betAmount;
    }

    /*
     * Returns amount of chips that are raised/called, but not yet moved to the pot.
     */ 
    public Decimal Bet {
        get {
            return bet;
        }
    }


    public List<Card> Cards
    {
        get
        {
            return cards;
        }
        set
        {
            cards = new List<Card>(value);
        }
    }
    /*
     * This method is called when chips are moved from bet to pot. 
     * GetBet method should be called before this.
     */ 
    internal void ResetBet()
    {
        bet = 0;
    }

    internal void AddToStack(decimal wonSize)
    {
        stack += wonSize;
    }

    internal void Reset()
    {
        cards = null;
        bet = 0;
    }

    public override int GetHashCode()
    {
        return id;
    }

    public override string ToString()
    {
        string cardsStr = (Cards != null) ? string.Join(",", cards) : "None";

        return "Player: " + Name + ", Cards: " + cardsStr
            + ", Stack: " + Stack.ToString() + ", Bet: " + Bet.ToString();
    }
}
}

Answer 1

Methods/Properties headers in C# starts with ///. Try it in Visual Studio, you'll see that it's much better, plus it generates the header template for you!

Also, you have too much comments. Usually, try to comment explaining why you did something, not how. That, people will figure it out quite easily. I can understand that you want method headers, but properties headers should rarely be longer than a few words and fields shouldn't be commented (not even headers) unless it it really important.

Watch your indentation, some of your code is hard to look at because there's not enough spacing. In C#, braces are on a line each, which means :

void methodA(){
}

becomes

void methodA()
{
}

It is somehow easier to read and respects the coding standards.

Try to keep your constants at the top of your file and keep your order consistent. Let's say you have this template :

• constants
• fields
• properties
• constructors
• methods

Try and keep it the same way everywhere. (This order is out of my hat, it doesn't exactly respect the C# coding standards if I'm not mistaken).

Answer 2

All methods including private methods should be pascal casing according to c# coding conventions.

In some places you have the wrong casing:

//Should be AddPlayer
public void addPlayer(int seat, Player player)

//Should be GetSelected and IncrementCounter
playersInHand.getSelected().MakeABet(SmallBlind);
playersInHand.incrementCounter();
playersInHand.getSelected().MakeABet(BigBlind);
playersInHand.incrementCounter();

//Should be DealCards
private void dealCards()
{
    foreach (Player player in playersInHand)
    {
        player.Cards = deck.DrawCards(CARDS_PER_PLAYER);
    }
}

This question is about pascal casing of private methods: https://stackoverflow.com/questions/2758684/code-style-for-private-methods-in-c-sharp

Answer 3

As they've already been covered by other reviewers, I won't go over the comments or the casing.

public PokerTable(Decimal smallBlind, Decimal bigBlind, uint maxPlayers)
{
    if (bigBlind / 2 < smallBlind) throw new ArgumentException("Big blind must be twice as big as small blind");
    if (smallBlind <= 0) throw new ArgumentException("Small blind must be positive");
    if (bigBlind <= 0) throw new ArgumentException("Big blind must be positive");

    SmallBlind = smallBlind;
    BigBlind = bigBlind;
    MaxPlayers = maxPlayers;
}

Checking if bigBlind is less than or equal to zero is pointless because you already know it's at least twice as big as smallBlind and you've just checked that is greater than 0.

I think you're missing a check on maxPlayers. Surely that has to be >= 2.

If the argument is outside of the allowed range throw an ArgumentOutOfRangeException as that's more descriptive.

---

public override string ToString()
{
    string cardsStr = (Cards != null) ? string.Join(",", cards) : "None";

    return "Player: " + Name + ", Cards: " + cardsStr
        + ", Stack: " + Stack.ToString() + ", Bet: " + Bet.ToString();
}

You should prefer string.Format or string interpolation (in C#6)

public override string ToString()
{
    string cardsStr = (Cards != null) ? string.Join(",", cards) : "None";

    return $"Player: {Name}, Cards: {cardsStr}, Stack: {Stack}, Bet: {Bet}";
}

---

public void StartHand()
{
    if(playersAtTable.Count < 2) throw new LackOfPlayersException();

I think an InvalidOperationException is perfect for that rather than requiring a custom exception type.

---

Now, why is your Poker Game also a Poker Table and a TableAction? It seems to be an odd inheritance structure to me.