Some years ago I made a Monty Hall simulations in Java, and now I decided to give it a go in Python, after learning (yet another) the language at university.
I decided to make a generalized, object-oriented and modularized version of the game, in order to see what alterations would impact the results; and to get more comfortable with Object-Oriented code.
I mainly have two questions, although all tips, ideas and hints are welcome:
1) Would I benefit from making Player
a subclass of Game (or the other way around)?
2) How would I best analyze the results from my code? I thought about initializing the variables with a numpy
range, but I'm not used to working with it. I could then print the results on a graph with mathplotlib
, I guess.
import random
NUMBER_OF_GAMES = 10000
P_SWAP = False
P_CAN_SELECT_SAME = False
N_DOORS = 5
N_PRIZES = 3
N_PLAYERS = 1
VERBOSE = True
PRINT_CANDY = "="*64+"\n"
try:
assert isinstance(P_SWAP, bool)
assert N_DOORS > 0
assert N_PRIZES > 0
assert N_PLAYERS >= 0
assert isinstance(VERBOSE, bool)
except (AssertionError, TypeError):
print("Invalid value for initialisation values")
exit()
try:
assert N_PRIZES <= N_DOORS
except AssertionError:
print("Can't have more prizes than doors")
exit()
try:
assert N_PLAYERS < N_DOORS - N_PRIZES
except AssertionError:
print("Must have at least 2 less players than doors")
exit()
GIFT = ["CAR"]
COAL = ["GOAT", "POOP", "COAL", "CRAP"]
win_counter = 0
lose_counter = 0
# overriding print functions
def supress_prints():
"""
Since print() redirects strings to STDOUT, we can override
this to /dev/null, instead of terminal, and thus
avoid printing unnecessary information. Speeds up program too.
"""
import sys, os
if VERBOSE:
return
sys.stdout = open(os.devnull, 'w')
def restore_prints():
"""
Returns print() to its default value: STDOUT.
"""
import sys
sys.stdout = sys.__stdout__
class Door:
def __init__(self, game: "Game", content: str, is_open: bool=False):
self.content = content
self.is_open = is_open
self.index = len(game.doors) # works as long as no doors are deleted from the game
print(self)
def is_prize(self):
if self.content == GIFT:
return True
else:
return False
def is_open(self):
return self.is_open
def is_closed(self):
return not self.is_open
def open(self):
self.is_open = True
def close(self):
self.is_open = False
def __str__(self):
return ["Open " if self.is_open else "Closed "][0] + self.content + " @door " + str(self.index)
class Game:
def __init__(self,
swaps: bool=True,
number_of_doors: int=3,
number_of_prizes: int=1,
number_of_players: int=1,
players_can_collide: bool=False
)->"Game":
self.number_of_doors = number_of_doors
self.number_of_prizes = number_of_prizes
self.number_of_players = number_of_players
self.players_can_collide = players_can_collide
self.doors = list()
self.prizes = list()
self.open_doors = list()
# create a Player object for each allowed player in the game
print("Creating players...")
self.players = list()
while len(self.players) < number_of_players:
self.players.append(
Player(self, swaps)
)
print("Done!"); print()
# figure out what doors should contain each prize
while len(self.prizes) < number_of_prizes:
self.winning_door = None
while self.winning_door in self.prizes \
or self.winning_door == None:
self.winning_door = random.randrange(0,number_of_doors)
self.prizes.append(self.winning_door)
# create all the Door objects in the game, while adding
# their contents simultaneously
print("Creating doors...")
for new_door in range(number_of_doors):
if new_door in self.prizes:
self.doors.append(
Door(self, random.choice(GIFT))
)
else:
self.doors.append(
Door(self, random.choice(COAL))
)
print("Done!");
print("Game is configured!")
print(PRINT_CANDY)
def start_game(self):
#############
# round one #
#############
print("ROUND ONE: \n")
# let all players select a door
self.selected_doors = list()
for player in self.players:
selection = player.select_door()
# finding doors which can be opened: must not be selected, nor can't have a GIFT in it
self.removable_candidates = list()
for player in self.players:
for door in self.doors:
if door.is_closed():
if self.doors.index(door) not in self.selected_doors:
if door.content not in GIFT:
self.removable_candidates.append(door)
# clear selections for game state (still saved in Player instances)
self.selected_doors.clear()
print(PRINT_CANDY)
####
## TODO: maybe add some temporary stat printing here
####
#############
# round two #
#############
print("ROUND TWO: \n")
self.pandora_door = random.choice(self.removable_candidates) # a door with evil stuff in it
self.pandora_door.open()
print("Pandora door:", self.pandora_door)
self.open_doors.append(self.pandora_door) # open one COAL door
print()
# for all players that want to swap door, do so now
for player in self.players:
if player.swaps:
player.select_door(force_change=True)
print()
###############
# end of game #
###############
# print results
print("Final results!")
for player in self.players:
global win_counter, lose_counter
print("Player", player.index, end=": ")
if self.doors[player.selection].content in GIFT:
print("Won a", self.doors[player.selection].content, "at door", player.selection)
win_counter += 1
else:
print("Got some", self.doors[player.selection].content, "by selecting door", player.selection)
lose_counter += 1
class Player:
def __init__(self, game: Game, swaps: bool) -> "Player":
self.game = game # could possibly be called by with super(), defining Player as a subclass of Game
self.swaps = swaps
self.selection = None
self.index = len(game.players) # non-intuitive, but works as long as players are not removed from game
print("Created player", self.index)
def select_door(self, force_change: int=False) -> int:
"""
Allow player to choose any Door from the game which
is not opened, and depending on the settings, whether is not
already selected by another player.
"""
self.old_selection = self.selection
self.selection = None
while (self.selection in self.game.selected_doors and self.game.players_can_collide) \
or self.selection in self.game.open_doors \
or (self.selection == self.old_selection and force_change) \
or self.selection is None:
self.selection = random.randrange(0, self.game.number_of_doors)
self.game.selected_doors.append(self.selection)
print("Player", self.index, "selected door", self.selection)
return self.selection # helper, not required
supress_prints()
for _ in range(NUMBER_OF_GAMES):
Monty_Hall_Game = Game(swaps=P_SWAP, number_of_doors=N_DOORS, number_of_prizes=N_PRIZES, number_of_players=N_PLAYERS)
Monty_Hall_Game.start_game()
print(PRINT_CANDY*2)
print()
restore_prints()
win_percent = win_counter*100//(NUMBER_OF_GAMES*N_PLAYERS)
lose_percent = lose_counter*100//(NUMBER_OF_GAMES*N_PLAYERS)
print(f"""
Statistics:
{PRINT_CANDY}
With a total of {NUMBER_OF_GAMES} games, configured as:
Players: {N_PLAYERS}
Swapping: {P_SWAP}
Colliding: {P_CAN_SELECT_SAME}
Doors: {N_DOORS}
Prizes: {N_PRIZES}
{win_counter} wins \t ({win_percent}%),
{lose_counter} losses \t ({lose_percent}%)!
{PRINT_CANDY}""")
All in all, how can I improve my code? What makes you cringe?
1 Answer 1
You’ve got many errors in your code.
In class Door
, you set self.is_open
to False
, but door already contained the method def is_open(self)
, so you’ve overwritten the object’s method, and can no longer call door.is_open()
.
The method def is_open(self)
returned self.is_open
, which would be a method reference if it wasn’t overwritten above, so the method would have always return a "truthy" value.
In class Player
, you have def select_door(self, force_change: int=False)
. The parameter force_change
is given the type int
, with a default value of False
. Round peg in a square hole.
Your code is unnecessarily verbose:
if self.content == GIFT:
return True
else:
return False
Could be:
return self.content == GIFT
But GIFT
is a list of all of the prizes! Shouldn’t there just be one prize behind a door? Maybe:
return self.content in GIFT
How does your code actually work? Oh, right, you never call the method door.is_prize()
, just like you never call door.is_open()
. If you write a function, test it and make sure it works.
1) Would I benefit from making Player a subclass of Game (or the other way around)?
Use the "is-a" test. Is a player a game? I’d say "no". How about is a game a player? Again, I’d say "no". Unless the answer to one of those is a "yes", making something a subclass of another thing is a very bad idea.