2
\$\begingroup\$

To give a short contextual description: It is a poker game. I plan to add "cards" and a "pot"-object later on and then learn event-driven programming so that I can add a GUI. Essentially the player's placement in the game-objects "player" variable determines their seat.

My main questions are in regards to the placement of methods and the data of each object. Do you guys see any sort of future issues with e.g. betting being a player method, and not a round method? Or, that there is no round-method for turning players in-active other than end-round that turns all players inactive?

Other than that, pretty much any feedback is welcome, as long as it is backed up with a reasonable explanation of course. I am just trying to learn as many tricks as possible and "future-thinking".

Code:

import random
import copy
import importlib

def limit(num, minimum=1, maximum=255):
  """Limits input 'num' between minimum and maximum values.
  Default minimum value is 1 and maximum value is 255."""
  return max(min(num, maximum), minimum)

#A player class - contains name, amount of cash and active-states
class player:

    def __init__(self,name,initMoney):
        self.cash = initMoney
        self.chipsInPlay = 0 #How much has this player betted?
        self.name = name

        #Data Related To Active-States
        self.activeGame = 1 #Does it exist at all? Reasons for non-existance: No cash, no chips and no possibility of re-joining
        self.activeRound = 1 #Does it exist in this round? Reasons for 0: No cash, no chips, folding
        self.activeTurn = 0 #Is it active this turn? Reasons for 0: Another player is active

    #If the player has no money and no stake in the pot, they're out of the game
    def checkIfOut(self):
        if self.cash == 0 and self.chipsInPlay == 0:
            self.activeRound = 0
            self.activeTurn = 0
            self.activeGame = 0

    #Checks if the player has enough money to bet the blind and bets - doesn't check activeTurn - Doesn't change: chipsInPlay,activeTurn
    def blindMoneyBet(self,blind):
        if blind<self.cash or blind == self.cash:
            self.cash = self.cash - blind
            print(f"\nPlayer: {self.name} just bet the blind of {blind}$\nCurrent balance: {self.cash}$\nCurrent money in pot: {self.chipsInPlay}$\n")
            return
        else:
            print("blind()-Player: Imagine an all-in function here")

    #Checks if the player has enough to bet and bets money for the player, the money goes into the pot
    #Checks: If allActiveStates = 1 - Changes: activeTurn = 0*,chipsInPlay = +bet - *: unless endTurn = 0
    def regularMoneyBet(self,moneyChange,min =0,endTurn = 1):
        if self.activeGame == 1 and self.activeRound == 1 and self.activeTurn == 1:
            betLimit = limit(moneyChange,min,self.cash)
            if moneyChange == betLimit: #He cannot bet more, than he has
                self.cash = self.cash - moneyChange
                self.chipsInPlay = self.chipsInPlay + moneyChange
                print(f"\nPlayer: {self.name} just bet {moneyChange}$\nCurrent balance: {self.cash}$\nCurrent money in pot: {self.chipsInPlay}$\n")
                if endTurn == 1:
                    self.activeTurn = 0
            elif moneyChange != betLimit:
                print(f"{self.name} tried to bet: {moneyChange}$, while he only has: {self.cash}$")
        elif self.activeGame != 0 or self.activeRound != 0 or self.activeTurn != 0:
            print(f"Player: {self.name} is not active in the following ways:\nGame: {self.activeGame}\nRound: {self.activeRound}\nTurn: {self.activeTurn}")

    #Turns activeRound = 0
    def fold(self):
        self.activeRound = 0
        print(f"{self.name} just folded")

    def __str__(self):
            return f"\nName: {self.name} \nCash left: {self.cash}\nMoney in pot: {self.chipsInPlay}\nGame active = {self.activeGame}\nRound active = {self.activeRound}\nTurn active = {self.activeTurn}"


#Contains previous turns
class gameRound:
    def __init__(self,players,startingPlayer = 0,bigBlind = 0, smallBlind = 0, tableSize = 5):

        #Data related to players in the round
        self.startingPlayer = startingPlayer #Given by the game object at initialization (Object)
        self.roundPlayers = players #Players who're still in play in the round (list with player objects)
        self.playersActiveRound = len(players) #Amount of players in the game (integer)

        #Data related to turns
        self.turns = []
        self.lastTurnType = 0 #For keeping tack of possible actions (Integer)
        self.latestBet = 0 #The last bet, that was made in this round - To decide how much to raise or check (integer)
        self.turnNumber = 0 #For keeping track of which turn number, this is (integer)

        #Data related to who is active this turn
        self.activeThisTurn = 0 #Which player is active (object)
        self.activeThisTurnNumber = 0 #What number in the list of active players is the current active player (integer)
        self.playersActiveTurn = 0 #How many players have self.activeTurn = 1 - used for debugging (integer)

        #Data related to initial setup
        self.bigBlind = bigBlind #The bet that the first activeThisTurn has to bet (integer)
        self.smallBlind = smallBlind #The bet that roundPlayers[activeThisTurnNumber -1] has to bet i.e Player to the left of BBL. (integer)
        self.saveTurn() #Saves the initial player data at turn X (list with player objects)
        self.tableSize = tableSize

    #Debug methods below

    #Finds how many players are active - integer, not the actual player objects
    def findPlayersActiveTurn(self):
        g = 0
        for x in self.roundPlayers:
            if x.activeTurn == 1:
                g += g
        self.playersActiveTurn = g

    #Sets the person who is active this turn, and sets the previous active player as inactive (turn)
    def setActiveTurn(self,playerName): #Startingplayer, which is the optional argument in the game object, which is passed down into playerName is by default 0
        if type(self.activeThisTurn) == player:
            self.activeThisTurn.activeTurn = 0

        if playerName == 0: #If no name is given
            x = self.roundPlayers[random.randint(0, self.playersActiveRound - 1)]
            self.activeThisTurn = x
            self.findActiveNumber()
            x.activeTurn = 1
        elif playerName != 0: #If a name is given
            for x in self.roundPlayers:
                if x.name == playerName:
                    x.activeTurn = 1
                    self.activeThisTurn = x

    #Saves the current player data as a nested list in the self.turns list
    def saveTurn(self):
        z = [] #For storing playerdata
        for x in self.roundPlayers:
            y = copy.copy(x) #Makes a copy of the player objects
            z.append(y)

        self.turns.append(0) #Adds a new index
        self.turns[self.turnNumber] = z #Turns this index into z

    #Finds the current active player's number in the turn order
    def findActiveNumber(self):
        g= -1 #List indexes start at 0
        for x in self.roundPlayers:
            g = g +1
            if x == self.activeThisTurn:
                self.activeThisTurnNumber = g
                #Make a debug such that, if there are more actives this turn, it will say so

    #Selects the closest roundActive player to the right of the current activeTurnPlayer as the next activeTurn.
    def nextActive(self):
        self.findActiveNumber()

        y = (self.activeThisTurnNumber+1)%len(self.roundPlayers) #Goes one player to the right, modulo so it restarts at 0 if y +1 is out of bounds

        for x in range(y,len(self.roundPlayers)): #x in [y;self.playersActiveRound[
            h = x%len(self.roundPlayers)
            self.roundPlayers[h].checkIfOut()

            if self.roundPlayers[h].activeRound == 1 and self.roundPlayers[h] != self.activeThisTurn: #First activeRound player to the right of this current active player
                self.roundPlayers[h].activeTurn = 1
                self.activeThisTurn = self.roundPlayers[h]
                return() #Ends it
            else:
                print(f"\nNo other active players than {self.activeThisTurn.name}")

    #Removes inactive players from the round list
    def removeInactiveRound(self):
        listOfActivePlayers = []
        for x in self.roundPlayers:
            x.checkIfOut()
            if x.activeRound == 1 and x.activeGame == 1:
                listOfActivePlayers.append(x)
        self.playersActiveRound = len(listOfActivePlayers)
        self.roundPlayers = listOfActivePlayers

    #Increments the turn by 1, changes the activeTurn player, removes inactive players and saves the player data to the turnlist
    def endTurn(self):
        self.turnNumber = self.turnNumber + 1
        self.nextActive()
        self.removeInactiveRound()
        self.saveTurn()

    def startingTurn(self):
        self.setActiveTurn(self.startingPlayer) #Starting player is provided by the game-object whenever a round is initialized
        self.activeThisTurn.blindMoneyBet(self.bigBlind) #Blind instead of moneybet, as there are no restrictions in terms of active status
        print(self.activeThisTurnNumber)
        self.roundPlayers[self.activeThisTurnNumber-1].blindMoneyBet(self.smallBlind) #This works because -1 = highest number, so if 0 -1 = -1 = highest index in the list


class game:
    def __init__(self,initPlayers,startingPlayer = 0,bigBlind = 25, smallBlind = 10):
        self.players = initPlayers
        self.updateValuesPlayersSum()
        self.playersCash = self.sumList[0] #How Much Money The Players Have excl. In Pot
        self.playersActiveGame = self.sumList[1] #How Many Players Are Active In The Game (int)
        self.chipsInPlay = self.sumList[2] #The Current Pot Size
        self.bigBlind = bigBlind
        self.smallBlind = smallBlind
        self.startingPlayer = startingPlayer #The initial starting player for this or the next round
        self.startRound() #Creates a gameRound object, chooses the initial starting player and makes the players bet BBL and SBL
        self.currentRound

    #Sums up the amount of cash held by all players and returns a float
    def cashPlayersSum(self,players):
        sum = 0
        for x in players:
            sum = x.cash + sum
        return sum

    #Sums the amount of active players and returns an integer
    def playersActiveGameSum(self,players):
        sum = 0
        for x in players:
            sum = x.activeGame + sum
        return sum

    #Sums the chips in play AKA the Pot and returns number
    def chipsInPlayPlayersSum(self,players):
        sum = 0
        for x in players:
            sum = x.chipsInPlay + sum
        return sum

    # Sums up all sum-values and adds them to a list
    def valuesPlayersSum(self,players):
        totalSum = []
        totalCash = self.cashPlayersSum(players)
        totalActivesGame = self.playersActiveGameSum(players)
        totalChipsInPlay = self.chipsInPlayPlayersSum(players)

        totalSum.append(totalCash)
        totalSum.append(totalActivesGame)
        totalSum.append(totalChipsInPlay)

        return totalSum

    #Updates the game's player-based sums
    def updateValuesPlayersSum(self):

        totalSum = self.valuesPlayersSum(self.players)

        self.sumList = totalSum
        self.playersCash = self.sumList[0]
        self.totalActivesGame = self.sumList[1]
        self.chipsInPlay = self.sumList[2]

    #Sets a person to be active in the round, and makes the first active player bet, and the player left to him on the list bet.
    def startRound(self):
        self.currentRound = gameRound(self.players,self.startingPlayer,self.bigBlind,self.smallBlind)

    def gameEndTurn(self):
        self.currentRound.endTurn()
    #Needs a function that continously allows for each active player to choose an action, unless all - 1 have folded, all have checked, or all have called the bet enough times to end round

    def __str__(self):
        return f"\nPlayers in game : {str(self.players)} \nActive players: {str(self.playersActiveGame)} \nPot size: {str(self.chipsInPlay)} \nCash on hand:  {str(self.playersCash)} "


def testNoob():
    player0 = player("player0",125)
    player1 = player("player1",125)
    player2 = player("player2",125)

    players = [player0,player1,player2]

    aGame = game(players)
    return aGame
\$\endgroup\$
3
  • 1
    \$\begingroup\$ The first program that I ever tried to write was an OO poker game. I remember gatting stuck trying to implement a truly random shuffle() method. There were plenty online, but I was overly proud and wanted to do it on my own. I was stuck for a few weeks then had a eureka moment working on something completely different. That is the first time I ever solved a problem that had me stumped. I'm feeling some crazy nostalgia right now. Can't help but smile. Sry for droning on : ) \$\endgroup\$
    – Nate T
    Jun 18, 2021 at 20:14
  • 1
    \$\begingroup\$ As a tip: write it in a way that lets you reuse. You can write card game and then sort of extend that. That way, if you want to make solitaire later, you're already halfway there. The first half could be an incredibly useful template for thousands of programs. Oh yea, and start workin on that shuffle method! The earlier the better! XD \$\endgroup\$
    – Nate T
    Jun 18, 2021 at 20:14
  • 2
    \$\begingroup\$ Adding to my earlier comments, as you are using python, you should write card game as a module, and include said module at the top of this one. This will give you the reuse effect that I mentioned above. There are many methods, such as deal() and shuffle() and many properties, like Card and Hand(which would be a list of Cards), that are common to just about every card game. Extract them into said module. Python has excellent modularity, imo. Furthermore, this use case will let you use it to your advantage in a big way. \$\endgroup\$
    – Nate T
    Jun 18, 2021 at 20:36

2 Answers 2

2
\$\begingroup\$

Welcome to CodeExchange. Here are some ways in which your code can be improved:

Code Style

Naming conventions

Please, use PEP-8 naming conventions. For example, all classes names have to start with an Uppercase letter:

game -> Game
player -> Player

In general, it seems like you follow them :)

Long lines

Do not write lines that are too long. It makes them hard to read. For example:

return f"\nPlayers in game : {str(self.players)} \nActive players: {str(self.playersActiveGame)} \nPot size: {str(self.chipsInPlay)} \nCash on hand:  {str(self.playersCash)} "

Is more readable as:

# By the way, there is no need to start the string representation of a class
# with an empty line. If what you want is to have an empty line before 
# the actual class string representation, that should be the caller's responsibility
return f"Players in game : {str(self.players)}\n"
    + f"Active players: {str(self.playersActiveGame)}\n"
    + f"Pot size: {str(self.chipsInPlay)}\n"
    + f"Cash on hand:  {str(self.playersCash)}"

Particularly, PEP-8 recommends making lines no longer than 80 characters. Some argue that is not enough (specially if you like to give you variables long meaningful names), but it is already a hint. For example , the line:

return f"\nName: {self.name} \nCash left: {self.cash}\nMoney in pot: {self.chipsInPlay}\nGame active = {self.activeGame}\nRound active = {self.activeRound}\nTurn active = {self.activeTurn}"

along with its indentation, is 202 characters long. Most people will not be able to fit such a long line on their screen (not to say it is not easy to read).

This same thing applies to comments. Its a really good practise to comment you code. However, if you plan on having long comments, write them above the statements, instead of next to them. For example:

self.setActiveTurn(self.startingPlayer) #Starting player is provided by the game-object whenever a round is initialized

Is more readable as such:

# Starting player is provided by the game-object whenever a round is 
# initialized
self.setActiveTurn(self.startingPlayer)

Consistent style

Be consistent in your code style.

For example, sometimes you leave spaces between operators and sometimes you don't:

# Asignments
g= -1 #List indexes start at 0
g = g +1
self.turnNumber = self.turnNumber + 1
sum = x.chipsInPlay + sum
y = (self.activeThisTurnNumber+1)%len(self.roundPlayers)
...
# Functions/methods
def limit(num, minimum=1, maximum=255):
def __init__(self,name,initMoney):
def regularMoneyBet(self,moneyChange,min =0,endTurn = 1):

Moreover, following PEP-8 recommendations, you should always (there are a few exceptions) leave a space between the operators and variables, as well after commas. So the previous statements/declarations would be better written as:

# Asignments
g = -1 #List indexes start at 0
g = g + 1
self.turnNumber = self.turnNumber + 1
sum = x.chipsInPlay + sum
y = (self.activeThisTurnNumber + 1) % len(self.roundPlayers)
...
# Functions/methods
def limit(num, minimum=1, maximum=255):
def __init__(self, name, initMoney):
def regularMoneyBet(self, moneyChange, min=0, endTurn=1):

Docstrings

Its a really good practise to document you methods/functions. Just use docstrings so that your comments can be used to produce documentation.

For example, if you documment a function like this:

# This is my documentation
def myFunction():
    pass

Then, you cannot access the documentation if its not by reading the code:

>>> help(myFunction)
Help on function myFunction in module __main__:

myFunction()

However, if you document it as this:

def myFunction():
    '''This is my documentation'''
    pass

Then:

>>> help()
Help on function myFunction in module __main__:

myFunction()
    This is my documentation

You can find more on docstrings here.

Design

Now, regarding code design, here are a few tips:

Native types are there for a reason!

Bools

Player.activeGame is always used as a variable with two states: 0 or 1. That is exactly what bools are for:

class Player:
    def __init__(self,name,initMoney):
        ...

        self.activeGame = True
        self.activeRound = True
        self.activeTurn = False

Going one step ahead, its a better idea to name boolean values using verbs:

class Player:
    def __init__(self,name,initMoney):
        ...

        # Some names that might be more meaningful
        self.isInGame = True
        self.isPlayingCurrentRound = True
        self.isTheirTurn = False

Now you can change you conditions, such as:

if self.activeGame == 1 and self.activeRound == 1 and self.activeTurn == 1
    ...

to simply:

if self.isInGame and self.isPlayingCurrentRound and self.isTheirTurn:
    ...

If you then wonder, for example, how you could get the total number of players in game, there are always solutions to problems. For example:

# Instead of this
def playersActiveGameSum(self, players):
    sum = 0
    for x in players:
        sum = x.activeGame + sum
    return sum

# You could use a more idiomatic expression
 def playersActiveGameSum(self, players):
    return sum([1 for player in players if player.isInGame])

Dicts

If you plan on having seemingly different data returned in a function, such as here:

def valuesPlayersSum(self, players):
    totalSum = []
    totalCash = self.cashPlayersSum(players)
    totalActivesGame = self.playersActiveGameSum(players)
    totalChipsInPlay = self.chipsInPlayPlayersSum(players)

    totalSum.append(totalCash)
    totalSum.append(totalActivesGame)
    totalSum.append(totalChipsInPlay)

    return totalSum

You could use a dict, so that accessing the returned values has more semantics:

def valuesPlayersSum(self, players):
    playerStatistics = {}

    playerStatistics['totalCash'] = self.cashPlayersSum(players)
    playerStatistics['totalActivesGame'] = self.playersActiveGameSum(players)
    playerStatistics['totalChipsInPlay'] =  self.chipsInPlayPlayersSum(players)

    return totalSum

This way, you would access the returned value's data as such:

statistics = self.valuesPlayersSum(players)
print(f'Total cash: {statistics['totalCash']}')
print(f'Total active players: {statistics['totalActivesGame']}')
print(f'Total chips in play: {statistics['totalChipsInPlay']}')

Even better, make use of the shipped-by-default namedtuple in the collections module:

from collections import namedtuple
PlayerStatistics = namedtuple('PlayerStatistics', 
    ['totalCash', 'totalActivesGame', 'totalChipsInPlay'])

...

def valuesPlayersSum(self, players):
    totalCash = self.cashPlayersSum(players)
    totalActivesGame = self.playersActiveGameSum(players)
    totalChipsInPlay =  self.chipsInPlayPlayersSum(players)

    return PlayerStatistics(totalCash, totalActivesGame, totalChipsInPlay)

...

statistics = self.valuesPlayersSum(players)
print(f'Total cash: {statistics.totalCash}')
print(f'Total active players: {statistics.totalActivesGame}')
print(f'Total chips in play: {statistics.totalChipsInPlay}')

Duplicated code

Try to follow the DRY (Dont Repeat Yourself) principle. Specially, if you are going to be doing the same exact thing twice one after the other.

For example, in Game's constructor, you are doing the following asignments:

def __init__(self,initPlayers,startingPlayer = 0,bigBlind = 25, smallBlind = 10):
        ...
        self.updateValuesPlayersSum()
        self.playersCash = self.sumList[0]        # <- Here
        self.playersActiveGame = self.sumList[1]  # <- Here
        self.chipsInPlay = self.sumList[2]        # <- Here
        ...

When you have already done them within Game#updateValuesPlayersSum:

def updateValuesPlayersSum(self):
        totalSum = self.valuesPlayersSum(self.players)

        self.sumList = totalSum
        self.playersCash = self.sumList[0]      # <- Here
        self.totalActivesGame = self.sumList[1] # <- Here
        self.chipsInPlay = self.sumList[2]      # <- Here

Other comments

When submiting a program like this to be reviewed, it is easier if you provide executable code. That way, we can see the way you intend it to be run.

The code you have provided, if executed, does nothing other than initializing variables.

I guess this is already some feedback to begin with. If you wish to update your code, and provide some executable code in another question, I could probably review more of the design :)

\$\endgroup\$
1
  • \$\begingroup\$ Hello m-alorda, First off, thank you very much for using so much of your time to write such a detailed reccomendation/advice. Secondly, I do plan on building upon this. Just had to make sure, that the fundament didn't have any serious design errors or the like, such that it became a problem later on in the proccess. I will get back to you, once I have more :) Also, thank you very much for the PEP-link. I didn't even know that, that was a thing, but it is exactly what I was looking for. \$\endgroup\$ Jun 24, 2021 at 16:27
2
\$\begingroup\$

There's a bunch. In no particular order,

  • Consider including PEP484 type hints. For limit(), you can use a generic type parameter to enforce that the input and output are numbers, and the input and output number type (float, int, etc.) match.
  • class Player, title-case.
  • init_money and so on, lower_snake_case, for all of your variables and method names.
  • Prefixing parameters with init_ is not helpful.
  • active_game etc. should be bool and not int. Checking them should look like if self.active_game, not if self.active_game == 1
  • blind<self.cash or blind == self.cash should just be blind <= self.cash
  • There are a few like this, but self.cash = self.cash - blind should be self.cash -= blind
  • Your convention of nnnn$ is odd, and at least in North-American culture is more commonly $nnn. The more general thing to do is not add the currency symbol at all, and call into locale.currency() instead.
  • Inline foo: #some comment should be foo: # some comment
  • You seem to be assigning 0 to indicate that an object reference, i.e. startingPlayer, is "empty". This should be replaced with None and assigned the Optional type hint.
  • start_round() should not be called from __init__. The core game logic should be executed in its own method; __init__ is only for initialization. Furthering that point: updateValuesPlayersSum is setting new variables that do not appear in __init__, which is ungood.
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.