2
\$\begingroup\$

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?

AlexV
7,3532 gold badges24 silver badges47 bronze badges
asked Jul 7, 2019 at 18:27
\$\endgroup\$

1 Answer 1

5
\$\begingroup\$

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.

answered Jul 8, 2019 at 5:02
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.