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
2 Answers 2
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 bool
s 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 :)
-
\$\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\$GeorgeWTrump– GeorgeWTrump2021年06月24日 16:27:13 +00:00Commented Jun 24, 2021 at 16:27
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 bebool
and notint
. Checking them should look likeif self.active_game
, notif self.active_game == 1
blind<self.cash or blind == self.cash
should just beblind <= self.cash
- There are a few like this, but
self.cash = self.cash - blind
should beself.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 intolocale.currency()
instead. - Inline
foo: #some comment
should befoo: # some comment
- You seem to be assigning
0
to indicate that an object reference, i.e.startingPlayer
, is "empty". This should be replaced withNone
and assigned theOptional
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.
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\$card game
as a module, andinclude
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\$