Python Bingo game that stores card in a dictionaryFind Missing Numbers in an int listWar card game simulatorApproximate (250 over 100) permutation best fitting certain criteriaCustom card gameGuess number gameA simple lotto game in PythonSimple game of lotto in Python v 2.0Randomly pick key from dictionary weighted by the keys valuePython function to get numbers around another numberAggregating recent trading data from Bitcoin derivatives exchange
Is it okay to true (align) a wheel with a missing spoke?
How do I compile something for Linux if I don't have enough space for installing GCC?
Sleep for 1000 years
What is the mathematical formula for proficiency bonus vs level/CR?
How to teach children Santa is not real, while respecting other kids beliefs?
Can I leave my car sitting outside for about 5 years?
What are the downsides of being a debt-free country (no national debt?
What is a logic gate?
Why doesn't knowledge of how magic works break magic in this world?
Conversion of mass into energy with 100% efficiency
What does it mean to play in a mode?
What's the difference between xxxx-client and xxxx-server packages?
Home rebuild and demolish
Fourier transform is an isomorphism...but we don’t get when each frequency appears?
Can I ignore an open source license if I checkout a version that was released prior to the license being added
Do any countries have a pensions system funded entirely by past contributions, rather than current taxes?
Why is marginal cost not the cost of producing the last unit?
How can conflict be conducted between nations when warfare is never an option?
How to find maximum amperage need for fuse
"Dog" can mean "something of an inferior quality". What animals do we use, if any, to describe the opposite?
Should I replace fillable PDFs?
Was there really a shuttle toilet training device with a "boresight camera"?
Is rotating a pawn so that it faces a different direction and then moves in that direction technically permitted according to the 2018 FIDE Laws?
Exponent like 2^3 in SI
Python Bingo game that stores card in a dictionary
Find Missing Numbers in an int listWar card game simulatorApproximate (250 over 100) permutation best fitting certain criteriaCustom card gameGuess number gameA simple lotto game in PythonSimple game of lotto in Python v 2.0Randomly pick key from dictionary weighted by the keys valuePython function to get numbers around another numberAggregating recent trading data from Bitcoin derivatives exchange
.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty,.everyoneloves__bot-mid-leaderboard:empty
margin-bottom:0;
$begingroup$
I am super proud of this python bingo game I wrote. I am posting it here to make get some help reviewing it to make sure I'm using the best practices. This is for a class, and my professor is a stickler for using best practices. I went above and beyond what the assignment called for so I need a little more helping making sure the code is okay. Any help is appreciated!
Here is the code:
# Author: Helana Brock
# Homework 2, problem 3
import random
random_draw_list = random.sample(range(1, 76), 75)
def generate_card():
"""
Generates a bingo card and stores the numbers in a dictionary.
"""
card =
"B": [],
"I": [],
"N": [],
"G": [],
"O": [],
min = 1
max = 15
for letter in card:
card[letter] = random.sample(range(min, max), 5)
min += 15
max += 15
if letter == "N":
card[letter][2] = "X" # free space!
return card
def print_card(card):
"""
Prints the bingo card.
Arguments:
card (dictionary): The card to be printed out.
"""
for letter in card:
print(letter, end="t")
for number in card[letter]:
print(number, end="t")
print("n")
print("n")
def draw(card, list):
"""
Pops a number off a list of random numbers. Using the pop method ensures no duplicate
numbers will be drawn.
Arguments:
card (dictionary): The card to to check for the number that was drawn.
list (list): The list of random numbers to be drawn from.
"""
number_drawn = random_draw_list.pop()
for letter in card:
i = 0
for number in card[letter]:
if number == number_drawn:
card[letter][i] = "X"
i += 1
return number_drawn
def check_win(card):
"""
First checks for diagonal wins, then four-corner, then horizontal, and finally, vertical.
Arguments:
card (dictionary): The card to check for a win.
"""
win = False
if card["B"][0] == "X" and card["I"][1] == "X" and card["N"][2] == "X" and card["G"][3] == "X" and card["O"][4] == "X":
win = True
elif card["O"][0] == "X" and card["G"][1] == "X" and card["N"][2] == "X" and card["I"][3] == "X" and card["B"][4] == "X":
win = True
elif card["B"][0] == "X" and card["O"][4] == "X" and card["B"][4] == "X" and card["O"][0] == "X":
win = True
for letter in card:
if(len(set(card[letter]))==1):
win = True
for i in range(5):
cnt = 0
for letter in card:
if card[letter][i] == "X":
cnt += 1
print(cnt)
if cnt == 5:
win = True
break
return win
def main():
"""
Main method for the program. Generates a card, prints it, and loops through drawing
and printing until the check_win method returns True or the user enters "quit".
"""
print("Let's play bingo!")
card = generate_card()
print("nHere is your card:n")
print_card(card)
print("nKeep pressing enter to continue or type quit to exit.n")
user_input = input()
win = check_win(card)
balls_till_win = 0
while win == False and user_input != "quit":
number_drawn = draw(card, random_draw_list)
balls_till_win += 1
print(f"nNumber drawn: number_drawn.")
print(f"Total balls drawn: balls_till_win.n")
print_card(card)
win = check_win(card)
user_input = input()
if win == True:
print(f"nBingo! Total balls to win: balls_till_win.")
else:
print("Goodbye! Thanks for playing!")
main()
python game homework
$endgroup$
add a comment
|
$begingroup$
I am super proud of this python bingo game I wrote. I am posting it here to make get some help reviewing it to make sure I'm using the best practices. This is for a class, and my professor is a stickler for using best practices. I went above and beyond what the assignment called for so I need a little more helping making sure the code is okay. Any help is appreciated!
Here is the code:
# Author: Helana Brock
# Homework 2, problem 3
import random
random_draw_list = random.sample(range(1, 76), 75)
def generate_card():
"""
Generates a bingo card and stores the numbers in a dictionary.
"""
card =
"B": [],
"I": [],
"N": [],
"G": [],
"O": [],
min = 1
max = 15
for letter in card:
card[letter] = random.sample(range(min, max), 5)
min += 15
max += 15
if letter == "N":
card[letter][2] = "X" # free space!
return card
def print_card(card):
"""
Prints the bingo card.
Arguments:
card (dictionary): The card to be printed out.
"""
for letter in card:
print(letter, end="t")
for number in card[letter]:
print(number, end="t")
print("n")
print("n")
def draw(card, list):
"""
Pops a number off a list of random numbers. Using the pop method ensures no duplicate
numbers will be drawn.
Arguments:
card (dictionary): The card to to check for the number that was drawn.
list (list): The list of random numbers to be drawn from.
"""
number_drawn = random_draw_list.pop()
for letter in card:
i = 0
for number in card[letter]:
if number == number_drawn:
card[letter][i] = "X"
i += 1
return number_drawn
def check_win(card):
"""
First checks for diagonal wins, then four-corner, then horizontal, and finally, vertical.
Arguments:
card (dictionary): The card to check for a win.
"""
win = False
if card["B"][0] == "X" and card["I"][1] == "X" and card["N"][2] == "X" and card["G"][3] == "X" and card["O"][4] == "X":
win = True
elif card["O"][0] == "X" and card["G"][1] == "X" and card["N"][2] == "X" and card["I"][3] == "X" and card["B"][4] == "X":
win = True
elif card["B"][0] == "X" and card["O"][4] == "X" and card["B"][4] == "X" and card["O"][0] == "X":
win = True
for letter in card:
if(len(set(card[letter]))==1):
win = True
for i in range(5):
cnt = 0
for letter in card:
if card[letter][i] == "X":
cnt += 1
print(cnt)
if cnt == 5:
win = True
break
return win
def main():
"""
Main method for the program. Generates a card, prints it, and loops through drawing
and printing until the check_win method returns True or the user enters "quit".
"""
print("Let's play bingo!")
card = generate_card()
print("nHere is your card:n")
print_card(card)
print("nKeep pressing enter to continue or type quit to exit.n")
user_input = input()
win = check_win(card)
balls_till_win = 0
while win == False and user_input != "quit":
number_drawn = draw(card, random_draw_list)
balls_till_win += 1
print(f"nNumber drawn: number_drawn.")
print(f"Total balls drawn: balls_till_win.n")
print_card(card)
win = check_win(card)
user_input = input()
if win == True:
print(f"nBingo! Total balls to win: balls_till_win.")
else:
print("Goodbye! Thanks for playing!")
main()
python game homework
$endgroup$
add a comment
|
$begingroup$
I am super proud of this python bingo game I wrote. I am posting it here to make get some help reviewing it to make sure I'm using the best practices. This is for a class, and my professor is a stickler for using best practices. I went above and beyond what the assignment called for so I need a little more helping making sure the code is okay. Any help is appreciated!
Here is the code:
# Author: Helana Brock
# Homework 2, problem 3
import random
random_draw_list = random.sample(range(1, 76), 75)
def generate_card():
"""
Generates a bingo card and stores the numbers in a dictionary.
"""
card =
"B": [],
"I": [],
"N": [],
"G": [],
"O": [],
min = 1
max = 15
for letter in card:
card[letter] = random.sample(range(min, max), 5)
min += 15
max += 15
if letter == "N":
card[letter][2] = "X" # free space!
return card
def print_card(card):
"""
Prints the bingo card.
Arguments:
card (dictionary): The card to be printed out.
"""
for letter in card:
print(letter, end="t")
for number in card[letter]:
print(number, end="t")
print("n")
print("n")
def draw(card, list):
"""
Pops a number off a list of random numbers. Using the pop method ensures no duplicate
numbers will be drawn.
Arguments:
card (dictionary): The card to to check for the number that was drawn.
list (list): The list of random numbers to be drawn from.
"""
number_drawn = random_draw_list.pop()
for letter in card:
i = 0
for number in card[letter]:
if number == number_drawn:
card[letter][i] = "X"
i += 1
return number_drawn
def check_win(card):
"""
First checks for diagonal wins, then four-corner, then horizontal, and finally, vertical.
Arguments:
card (dictionary): The card to check for a win.
"""
win = False
if card["B"][0] == "X" and card["I"][1] == "X" and card["N"][2] == "X" and card["G"][3] == "X" and card["O"][4] == "X":
win = True
elif card["O"][0] == "X" and card["G"][1] == "X" and card["N"][2] == "X" and card["I"][3] == "X" and card["B"][4] == "X":
win = True
elif card["B"][0] == "X" and card["O"][4] == "X" and card["B"][4] == "X" and card["O"][0] == "X":
win = True
for letter in card:
if(len(set(card[letter]))==1):
win = True
for i in range(5):
cnt = 0
for letter in card:
if card[letter][i] == "X":
cnt += 1
print(cnt)
if cnt == 5:
win = True
break
return win
def main():
"""
Main method for the program. Generates a card, prints it, and loops through drawing
and printing until the check_win method returns True or the user enters "quit".
"""
print("Let's play bingo!")
card = generate_card()
print("nHere is your card:n")
print_card(card)
print("nKeep pressing enter to continue or type quit to exit.n")
user_input = input()
win = check_win(card)
balls_till_win = 0
while win == False and user_input != "quit":
number_drawn = draw(card, random_draw_list)
balls_till_win += 1
print(f"nNumber drawn: number_drawn.")
print(f"Total balls drawn: balls_till_win.n")
print_card(card)
win = check_win(card)
user_input = input()
if win == True:
print(f"nBingo! Total balls to win: balls_till_win.")
else:
print("Goodbye! Thanks for playing!")
main()
python game homework
$endgroup$
I am super proud of this python bingo game I wrote. I am posting it here to make get some help reviewing it to make sure I'm using the best practices. This is for a class, and my professor is a stickler for using best practices. I went above and beyond what the assignment called for so I need a little more helping making sure the code is okay. Any help is appreciated!
Here is the code:
# Author: Helana Brock
# Homework 2, problem 3
import random
random_draw_list = random.sample(range(1, 76), 75)
def generate_card():
"""
Generates a bingo card and stores the numbers in a dictionary.
"""
card =
"B": [],
"I": [],
"N": [],
"G": [],
"O": [],
min = 1
max = 15
for letter in card:
card[letter] = random.sample(range(min, max), 5)
min += 15
max += 15
if letter == "N":
card[letter][2] = "X" # free space!
return card
def print_card(card):
"""
Prints the bingo card.
Arguments:
card (dictionary): The card to be printed out.
"""
for letter in card:
print(letter, end="t")
for number in card[letter]:
print(number, end="t")
print("n")
print("n")
def draw(card, list):
"""
Pops a number off a list of random numbers. Using the pop method ensures no duplicate
numbers will be drawn.
Arguments:
card (dictionary): The card to to check for the number that was drawn.
list (list): The list of random numbers to be drawn from.
"""
number_drawn = random_draw_list.pop()
for letter in card:
i = 0
for number in card[letter]:
if number == number_drawn:
card[letter][i] = "X"
i += 1
return number_drawn
def check_win(card):
"""
First checks for diagonal wins, then four-corner, then horizontal, and finally, vertical.
Arguments:
card (dictionary): The card to check for a win.
"""
win = False
if card["B"][0] == "X" and card["I"][1] == "X" and card["N"][2] == "X" and card["G"][3] == "X" and card["O"][4] == "X":
win = True
elif card["O"][0] == "X" and card["G"][1] == "X" and card["N"][2] == "X" and card["I"][3] == "X" and card["B"][4] == "X":
win = True
elif card["B"][0] == "X" and card["O"][4] == "X" and card["B"][4] == "X" and card["O"][0] == "X":
win = True
for letter in card:
if(len(set(card[letter]))==1):
win = True
for i in range(5):
cnt = 0
for letter in card:
if card[letter][i] == "X":
cnt += 1
print(cnt)
if cnt == 5:
win = True
break
return win
def main():
"""
Main method for the program. Generates a card, prints it, and loops through drawing
and printing until the check_win method returns True or the user enters "quit".
"""
print("Let's play bingo!")
card = generate_card()
print("nHere is your card:n")
print_card(card)
print("nKeep pressing enter to continue or type quit to exit.n")
user_input = input()
win = check_win(card)
balls_till_win = 0
while win == False and user_input != "quit":
number_drawn = draw(card, random_draw_list)
balls_till_win += 1
print(f"nNumber drawn: number_drawn.")
print(f"Total balls drawn: balls_till_win.n")
print_card(card)
win = check_win(card)
user_input = input()
if win == True:
print(f"nBingo! Total balls to win: balls_till_win.")
else:
print("Goodbye! Thanks for playing!")
main()
python game homework
python game homework
edited Sep 29 at 20:33
AlexV
6,1572 gold badges15 silver badges40 bronze badges
6,1572 gold badges15 silver badges40 bronze badges
asked Sep 29 at 20:20
Helana BrockHelana Brock
1439 bronze badges
1439 bronze badges
add a comment
|
add a comment
|
4 Answers
4
active
oldest
votes
$begingroup$
Style
Python comes with an "official" Style Guide for Python Code (aka PEP 8) which is a widely accepted standard for naming conventions and the like. Since you say your professor is keen on using best practices, I'd recommend to have a look at those guidelines. There is also a variety of tools that can help you to check for those best practices automatically. A non-exhaustive list (including Pylint) can be found in this meta-post here on Code Review.
From what I have seen, your code looks quite good in that regard. I especially like your docstrings. The most prevalent issue in your code would be that there are usually two blank lines between different function definitions in order to make their separation more clear.
Code
Idiomatic Python
As @nz_21 already mentioned, if sth == True:
and also if sth == False:
can be expressed in a more idiomatic way as if sth:
and if not sth:
(Note: Pylint would also catch this particular "mistake".)
Redefining built-ins
Some of the variable names you have chosen (min
, max
, list
) redefine built-in functions from the Python core. You should avoid that. The simplest way is to append a single trailing underscore, e.g. list_
, in order to avoid that issue. Also the parameter list
seems to be unused in draw(...)
.
Random number generation
card[letter] = random.sample(range(min_, max_), 5)
could also be rewritten as card[letter] = [random.randrange(min_, max_) for _ in range(5)]
. Although I don't think that it will make a big difference in your case. The documentation explicitly list them as alternatives:
random.randrange(start, stop[, step])
Return a randomly selected element from range(start, stop, step). This
is equivalent to choice(range(start, stop, step)), but doesn’t
actually build a range object.
From that it seems that the later approach would make more sense for a large range of values to choose from.
Edit: removed after hint in the comments with regard to sampling with and without replacement. Thanks to @Wombatz.
Global variables
random_draw_list
is used as a global variable, but unfortunately not as a constant as one might expect, but as a mutable global variable. That makes it harder to reuse code. Since draw(...)
is the only function using this variable, it makes no sense to have it as a global variable. Instead, it should defined in main()
after you have changed draw(...)
to accept it as parameter (maybe that's what list
was supposed to do in the first place?) and passed when calling draw(...)
from main()
.
Top-level script environment
In order to show what part of the file is actually supposed to be run as a script, Python defines the concept of a top-level script environment. Although the name sounds daunting at first, it's quite likely that you have seen it in someone else's Python code: if __name__ == "__main__":
This tells the Python interpreter to only run that part of code if the file is used as a script. In your case all there is to do is to do add the above right before calling the main
-function:
if __name__ == "__main__":
main()
That also helps you to avoid the issue that Python always starts a new bingo game whenever you try to import
a function from your file ;-)
$endgroup$
$begingroup$
Oh my gosh! Thank you so much for your awesome answer. I will definitely adopt all of this into my code!
$endgroup$
– Helana Brock
Sep 30 at 12:30
1
$begingroup$
random.randrange
is not an alternative torandom.sample
and neither does the documentation say so. The alternative torandom.randrange
israndom.choice(range(...))
. The thing is:random.sample
samples without replacement whilerandom.choices
and your list comprehension sample with replacement. In other words: your listcomprehension can include duplicates whilerandom.sample
can not.
$endgroup$
– Wombatz
Sep 30 at 16:53
$begingroup$
@Wombatz: I "invalidated" that part of the answer. Thanks for pointing it out.
$endgroup$
– AlexV
Sep 30 at 18:05
add a comment
|
$begingroup$
Initializing your bingo card
You don't need to initialize your dictionary with lists, since random.sample
returns a list by default. Just iterate through the string "BINGO"
to set your keys. Also, the if
check doesn't need to happen, you know that you will be replacing the 3rd element of the list at the key 'N'
every time, so just have that be an instruction at the end:
def generate_card():
"""
Generates a bingo card and stores the numbers in a dictionary.
"""
# just start with a plain dictionary, random.sample
# returns a new list, so no need to pre-allocate lists
# you won't be using in the future
card =
# min and max are builtin functions, so use
# underscores to avoid name shadowing
_min = 1
_max = 15
for letter in 'BINGO':
card[letter] = random.sample(range(_min, _max), 5)
_min += 15
_max += 15
# You know which letter this needs to be, no need for the if block
card["N"][2] = "X" # free space!
return card
Checking the win condition
To check over the diagonals, you can use zip
for your keys and the indices to validate. For horizontals iterate over card.values()
and check all(item == 'X' for item in row)
. For columns, you can zip together the rows using argument unpacking.
By iterating over a dictionary, it returns the keys by default, which is why enumerate(dict)
is the desired structure here. To check the opposite direction, zip(reversed(range(5)), card)
will give you (idx, key)
pairs in opposite order in a pythonic way, since dict
cannot be reversed:
# down to the right
# iterating over the dictionary yields the keys
if all(card[k][idx]=='X' for idx, key in enumerate(card)):
print('win!')
return True
# up to the right
elif all(card[k][idx]=='X' for idx, key in zip(reversed(range(5)), card)):
print('win!')
return True
# horizontal condition
for row in card.values():
if all(item=='X' for item in row):
return True
# vertical condition
for column in zip(*card.values()):
if all(item=='X' for item in column):
return True
To show how this works:
import random
d = k: random.sample(range(10), 3) for k in 'abc'
# 'a': [5, 3, 7], 'b': [1, 8, 7], 'c': [4, 5, 8]
# diagonal
for idx, key in enumerate(d):
print(key, d[key][idx])
a 5
b 8
c 8
# opposite diagonal
for idx, key in zip(reversed(range(3)), d):
print(key, d[key][idx])
a 7
b 8
c 4
# rows
for row in d.values():
print(row)
[5, 3, 7]
[1, 8, 7]
[4, 5, 8]
for col in zip(*d.values()):
print(col)
(5, 1, 4)
(3, 8, 5)
(7, 7, 8)
Also, the return True
on all of the conditions means that the function will stop once a winning condition is hit, rather than going through all of the conditions when you really don't need to. This is called short-circuiting, meaning that you only evaluate up to the desired condition.
Looking at this a bit further, I'd probably refactor the row and column check into a different function:
def check_line(values):
"""
values is an iterable over either rows or columns
of the card. Should be iterable(str)
returns boolean
"""
for line in values:
if all(val=='X' for val in values):
return True
# the last two loops then look like:
elif check_line(card.values()):
return True
elif check_line(zip(*card.values())):
return True
return False
Class?
The fact that you're passing card
around to a lot of functions says (to me) that you could probably use a class here. The generate_card
method can be done on __init__
, and everything else takes a self
rather than a card
:
class BingoCard:
def __init__(self, _min=1, _max=15):
"""
Generates a bingo card and stores the numbers in a dictionary.
_min and _max are integers that default to 1 and 15
if a default game is desired
"""
# just start with a plain dictionary, random.sample
# returns a new list, so no need to pre-allocate lists
# you won't be using in the future
self.card =
for letter in 'BINGO':
self.card[letter] = random.sample(range(_min, _max), 5)
_min += 15
_max += 15
# You know which letter this needs to be, no need for the if block
self.card["N"][2] = "X" # free space!
# __init__ doesn't return anything
# this makes your card printable
# and requires you return a string
def __str__(self):
return 'n'.join('t'.join((letter, *map(str, values))) for letter, values in self.card.items())
def draw(self, random_draw_list):
"""
Pops a number off a list of random numbers. Using the pop method ensures no duplicate
numbers will be drawn.
Arguments:
self (dictionary): The card to to check for the number that was drawn.
list (list): The list of random numbers to be drawn from.
"""
number_drawn = random_draw_list.pop()
for letter in self.card:
# don't track an index here, use enumerate
for i, number in enumerate(self.card[letter]):
if number == number_drawn:
self.card[letter][i] = "X"
return number_drawn
def check_win(self):
# down to the right
if all(self.card[k][idx]=='X' for idx, key in enumerate(self.card)):
print('win!')
return True
# up to the right
elif all(self.card[k][idx]=='X' for idx, key in zip(reversed(range(5)), self.card)):
print('win!')
return True
# horizontal condition
elif self.check_line(self.card.values()):
return True
# vertical condition
elif self.check_line(zip(*self.card.values())):
return True
return False
@staticmethod
def check_line(values):
"""
values is an iterable over either rows or columns
of the card. Should be iterable(str)
returns boolean
"""
for line in values:
if all(val=='X' for val in values):
return True
# then, your bingo card is an instance of this class
card = BingoCard()
card.check_win()
# False
The __str__
function is outlined in the python data model, but the gist of it is that __str__
allows you to redefine what the informal string representation of an object looks like.
random_draw_list
To make this work, I'd add it as a condition on your while
loop. You pop
elements off of it, but what happens when there's nothing else to pop
? You will get an IndexError
for attempting to pop
from an empty list. To make the game re-playable, I'd have the user choose to re-play, and generate that new list on each play, just as you re-create the bingo card on each play:
def generate_random_list():
return random.sample(range(1, 76), 75)
def main():
card = BingoCard()
# generate the draw list here, on each play of the game
# that way if you run out of balls, you can handle that by restarting
# the game
random_draw_list = generate_random_list()
print(f"This is your card:n card")
# you don't need the user input here, I think it might
# be better to include it elsewhere, before generating the
# card. Instead, keep going while there are numbers to draw
while random_draw_list:
number_drawn = card.draw(random_draw_list)
# this variable is better named balls_drawn
balls_drawn += 1
print(f"You drew number_drawn")
print(f"Total balls drawn: balls_drawn")
if check_win(card):
print(f"You won after drawing balls_drawn balls!")
break
# if you were to check for user input during the game,
# it would make more sense to do that after you've at least
# checked one round
keep_playing = input("Keep playing? (y/n)").strip().lower()
if keep_playing == 'n':
print("Ok, thanks for playing")
break
else:
print("Sorry, there are no more balls to draw")
# This is where you prompt the user to play:
if __name__ == "__main__":
while True:
play = input("Would you like to play bingo? (y/n)")
if play.strip().lower() == 'y':
main()
else:
break
$endgroup$
$begingroup$
Amazing answer! I like the class idea :) I will definitely refactor that and look at what you have here when I improve my code!
$endgroup$
– Helana Brock
Sep 30 at 12:31
$begingroup$
@AlexV fair point on the class name. In the comments on that code snippet, I do explain why_min
and_max
were changed :)
$endgroup$
– C.Nivs
Sep 30 at 13:06
$begingroup$
@HelanaBrock quick note, I changed the diagonal check condition sincedict
is not reversible
$endgroup$
– C.Nivs
Sep 30 at 13:55
add a comment
|
$begingroup$
Some low-hanging fruit:
if win == True
can be written as
if win:
Know that print
delimits by a newline by default; you don't need to explicitly add n
unless you want two newlines.
You can use classes to make card
a class on its own.
$endgroup$
$begingroup$
Thank you so much! I will change that, I didnt think about it. Also yes I wanted to two newlines haha
$endgroup$
– Helana Brock
Sep 30 at 12:29
add a comment
|
$begingroup$
Nice job. Here's two comments to add to what other's have said.
range() excludes the stop point
The built-in function range(start, stop, step)
does not include stop
in the items it returns. So in generate_card()
:
min_ = 1
max_ = 15
...
random.sample(range(min_, max_), 5)
only selects from the numbers 1, 2, 3, ..., 14. max_
should be 16. I sometimes write code with an explicit +1
like this:
random.sample(range(min_, max_+1), 5)
to remind myself that I intended to include max_
in the range.
Separation of concerns
When a function in a program does multiple unrelated things, it may make the program more difficult to debug, modify, or enhance. For example, draw()
gets the next numbered ball and adds it to the BINGO card. If a the player could have multiple bingo cards, each card would be drawing it's own random balls. Clearly, that wouldn't work. It would be better if the ball was drawn separately and then checked against each card.
$endgroup$
$begingroup$
I like the separation of concerns point a lot, +1. To drive that home, I'd argue thatdrawing
is a function of the game, not the card, so it should not be implemented at the card level. Instead, draw should take anumber
rather than anumber_list
as an argument, and that should be checked against the card
$endgroup$
– C.Nivs
Sep 30 at 16:00
add a comment
|
Your Answer
StackExchange.ifUsing("editor", function ()
StackExchange.using("externalEditor", function ()
StackExchange.using("snippets", function ()
StackExchange.snippets.init();
);
);
, "code-snippets");
StackExchange.ready(function()
var channelOptions =
tags: "".split(" "),
id: "196"
;
initTagRenderer("".split(" "), "".split(" "), channelOptions);
StackExchange.using("externalEditor", function()
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled)
StackExchange.using("snippets", function()
createEditor();
);
else
createEditor();
);
function createEditor()
StackExchange.prepareEditor(
heartbeatType: 'answer',
autoActivateHeartbeat: false,
convertImagesToLinks: false,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: null,
bindNavPrevention: true,
postfix: "",
imageUploader:
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/4.0/"u003ecc by-sa 4.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
,
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
);
);
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f229872%2fpython-bingo-game-that-stores-card-in-a-dictionary%23new-answer', 'question_page');
);
Post as a guest
Required, but never shown
4 Answers
4
active
oldest
votes
4 Answers
4
active
oldest
votes
active
oldest
votes
active
oldest
votes
$begingroup$
Style
Python comes with an "official" Style Guide for Python Code (aka PEP 8) which is a widely accepted standard for naming conventions and the like. Since you say your professor is keen on using best practices, I'd recommend to have a look at those guidelines. There is also a variety of tools that can help you to check for those best practices automatically. A non-exhaustive list (including Pylint) can be found in this meta-post here on Code Review.
From what I have seen, your code looks quite good in that regard. I especially like your docstrings. The most prevalent issue in your code would be that there are usually two blank lines between different function definitions in order to make their separation more clear.
Code
Idiomatic Python
As @nz_21 already mentioned, if sth == True:
and also if sth == False:
can be expressed in a more idiomatic way as if sth:
and if not sth:
(Note: Pylint would also catch this particular "mistake".)
Redefining built-ins
Some of the variable names you have chosen (min
, max
, list
) redefine built-in functions from the Python core. You should avoid that. The simplest way is to append a single trailing underscore, e.g. list_
, in order to avoid that issue. Also the parameter list
seems to be unused in draw(...)
.
Random number generation
card[letter] = random.sample(range(min_, max_), 5)
could also be rewritten as card[letter] = [random.randrange(min_, max_) for _ in range(5)]
. Although I don't think that it will make a big difference in your case. The documentation explicitly list them as alternatives:
random.randrange(start, stop[, step])
Return a randomly selected element from range(start, stop, step). This
is equivalent to choice(range(start, stop, step)), but doesn’t
actually build a range object.
From that it seems that the later approach would make more sense for a large range of values to choose from.
Edit: removed after hint in the comments with regard to sampling with and without replacement. Thanks to @Wombatz.
Global variables
random_draw_list
is used as a global variable, but unfortunately not as a constant as one might expect, but as a mutable global variable. That makes it harder to reuse code. Since draw(...)
is the only function using this variable, it makes no sense to have it as a global variable. Instead, it should defined in main()
after you have changed draw(...)
to accept it as parameter (maybe that's what list
was supposed to do in the first place?) and passed when calling draw(...)
from main()
.
Top-level script environment
In order to show what part of the file is actually supposed to be run as a script, Python defines the concept of a top-level script environment. Although the name sounds daunting at first, it's quite likely that you have seen it in someone else's Python code: if __name__ == "__main__":
This tells the Python interpreter to only run that part of code if the file is used as a script. In your case all there is to do is to do add the above right before calling the main
-function:
if __name__ == "__main__":
main()
That also helps you to avoid the issue that Python always starts a new bingo game whenever you try to import
a function from your file ;-)
$endgroup$
$begingroup$
Oh my gosh! Thank you so much for your awesome answer. I will definitely adopt all of this into my code!
$endgroup$
– Helana Brock
Sep 30 at 12:30
1
$begingroup$
random.randrange
is not an alternative torandom.sample
and neither does the documentation say so. The alternative torandom.randrange
israndom.choice(range(...))
. The thing is:random.sample
samples without replacement whilerandom.choices
and your list comprehension sample with replacement. In other words: your listcomprehension can include duplicates whilerandom.sample
can not.
$endgroup$
– Wombatz
Sep 30 at 16:53
$begingroup$
@Wombatz: I "invalidated" that part of the answer. Thanks for pointing it out.
$endgroup$
– AlexV
Sep 30 at 18:05
add a comment
|
$begingroup$
Style
Python comes with an "official" Style Guide for Python Code (aka PEP 8) which is a widely accepted standard for naming conventions and the like. Since you say your professor is keen on using best practices, I'd recommend to have a look at those guidelines. There is also a variety of tools that can help you to check for those best practices automatically. A non-exhaustive list (including Pylint) can be found in this meta-post here on Code Review.
From what I have seen, your code looks quite good in that regard. I especially like your docstrings. The most prevalent issue in your code would be that there are usually two blank lines between different function definitions in order to make their separation more clear.
Code
Idiomatic Python
As @nz_21 already mentioned, if sth == True:
and also if sth == False:
can be expressed in a more idiomatic way as if sth:
and if not sth:
(Note: Pylint would also catch this particular "mistake".)
Redefining built-ins
Some of the variable names you have chosen (min
, max
, list
) redefine built-in functions from the Python core. You should avoid that. The simplest way is to append a single trailing underscore, e.g. list_
, in order to avoid that issue. Also the parameter list
seems to be unused in draw(...)
.
Random number generation
card[letter] = random.sample(range(min_, max_), 5)
could also be rewritten as card[letter] = [random.randrange(min_, max_) for _ in range(5)]
. Although I don't think that it will make a big difference in your case. The documentation explicitly list them as alternatives:
random.randrange(start, stop[, step])
Return a randomly selected element from range(start, stop, step). This
is equivalent to choice(range(start, stop, step)), but doesn’t
actually build a range object.
From that it seems that the later approach would make more sense for a large range of values to choose from.
Edit: removed after hint in the comments with regard to sampling with and without replacement. Thanks to @Wombatz.
Global variables
random_draw_list
is used as a global variable, but unfortunately not as a constant as one might expect, but as a mutable global variable. That makes it harder to reuse code. Since draw(...)
is the only function using this variable, it makes no sense to have it as a global variable. Instead, it should defined in main()
after you have changed draw(...)
to accept it as parameter (maybe that's what list
was supposed to do in the first place?) and passed when calling draw(...)
from main()
.
Top-level script environment
In order to show what part of the file is actually supposed to be run as a script, Python defines the concept of a top-level script environment. Although the name sounds daunting at first, it's quite likely that you have seen it in someone else's Python code: if __name__ == "__main__":
This tells the Python interpreter to only run that part of code if the file is used as a script. In your case all there is to do is to do add the above right before calling the main
-function:
if __name__ == "__main__":
main()
That also helps you to avoid the issue that Python always starts a new bingo game whenever you try to import
a function from your file ;-)
$endgroup$
$begingroup$
Oh my gosh! Thank you so much for your awesome answer. I will definitely adopt all of this into my code!
$endgroup$
– Helana Brock
Sep 30 at 12:30
1
$begingroup$
random.randrange
is not an alternative torandom.sample
and neither does the documentation say so. The alternative torandom.randrange
israndom.choice(range(...))
. The thing is:random.sample
samples without replacement whilerandom.choices
and your list comprehension sample with replacement. In other words: your listcomprehension can include duplicates whilerandom.sample
can not.
$endgroup$
– Wombatz
Sep 30 at 16:53
$begingroup$
@Wombatz: I "invalidated" that part of the answer. Thanks for pointing it out.
$endgroup$
– AlexV
Sep 30 at 18:05
add a comment
|
$begingroup$
Style
Python comes with an "official" Style Guide for Python Code (aka PEP 8) which is a widely accepted standard for naming conventions and the like. Since you say your professor is keen on using best practices, I'd recommend to have a look at those guidelines. There is also a variety of tools that can help you to check for those best practices automatically. A non-exhaustive list (including Pylint) can be found in this meta-post here on Code Review.
From what I have seen, your code looks quite good in that regard. I especially like your docstrings. The most prevalent issue in your code would be that there are usually two blank lines between different function definitions in order to make their separation more clear.
Code
Idiomatic Python
As @nz_21 already mentioned, if sth == True:
and also if sth == False:
can be expressed in a more idiomatic way as if sth:
and if not sth:
(Note: Pylint would also catch this particular "mistake".)
Redefining built-ins
Some of the variable names you have chosen (min
, max
, list
) redefine built-in functions from the Python core. You should avoid that. The simplest way is to append a single trailing underscore, e.g. list_
, in order to avoid that issue. Also the parameter list
seems to be unused in draw(...)
.
Random number generation
card[letter] = random.sample(range(min_, max_), 5)
could also be rewritten as card[letter] = [random.randrange(min_, max_) for _ in range(5)]
. Although I don't think that it will make a big difference in your case. The documentation explicitly list them as alternatives:
random.randrange(start, stop[, step])
Return a randomly selected element from range(start, stop, step). This
is equivalent to choice(range(start, stop, step)), but doesn’t
actually build a range object.
From that it seems that the later approach would make more sense for a large range of values to choose from.
Edit: removed after hint in the comments with regard to sampling with and without replacement. Thanks to @Wombatz.
Global variables
random_draw_list
is used as a global variable, but unfortunately not as a constant as one might expect, but as a mutable global variable. That makes it harder to reuse code. Since draw(...)
is the only function using this variable, it makes no sense to have it as a global variable. Instead, it should defined in main()
after you have changed draw(...)
to accept it as parameter (maybe that's what list
was supposed to do in the first place?) and passed when calling draw(...)
from main()
.
Top-level script environment
In order to show what part of the file is actually supposed to be run as a script, Python defines the concept of a top-level script environment. Although the name sounds daunting at first, it's quite likely that you have seen it in someone else's Python code: if __name__ == "__main__":
This tells the Python interpreter to only run that part of code if the file is used as a script. In your case all there is to do is to do add the above right before calling the main
-function:
if __name__ == "__main__":
main()
That also helps you to avoid the issue that Python always starts a new bingo game whenever you try to import
a function from your file ;-)
$endgroup$
Style
Python comes with an "official" Style Guide for Python Code (aka PEP 8) which is a widely accepted standard for naming conventions and the like. Since you say your professor is keen on using best practices, I'd recommend to have a look at those guidelines. There is also a variety of tools that can help you to check for those best practices automatically. A non-exhaustive list (including Pylint) can be found in this meta-post here on Code Review.
From what I have seen, your code looks quite good in that regard. I especially like your docstrings. The most prevalent issue in your code would be that there are usually two blank lines between different function definitions in order to make their separation more clear.
Code
Idiomatic Python
As @nz_21 already mentioned, if sth == True:
and also if sth == False:
can be expressed in a more idiomatic way as if sth:
and if not sth:
(Note: Pylint would also catch this particular "mistake".)
Redefining built-ins
Some of the variable names you have chosen (min
, max
, list
) redefine built-in functions from the Python core. You should avoid that. The simplest way is to append a single trailing underscore, e.g. list_
, in order to avoid that issue. Also the parameter list
seems to be unused in draw(...)
.
Random number generation
card[letter] = random.sample(range(min_, max_), 5)
could also be rewritten as card[letter] = [random.randrange(min_, max_) for _ in range(5)]
. Although I don't think that it will make a big difference in your case. The documentation explicitly list them as alternatives:
random.randrange(start, stop[, step])
Return a randomly selected element from range(start, stop, step). This
is equivalent to choice(range(start, stop, step)), but doesn’t
actually build a range object.
From that it seems that the later approach would make more sense for a large range of values to choose from.
Edit: removed after hint in the comments with regard to sampling with and without replacement. Thanks to @Wombatz.
Global variables
random_draw_list
is used as a global variable, but unfortunately not as a constant as one might expect, but as a mutable global variable. That makes it harder to reuse code. Since draw(...)
is the only function using this variable, it makes no sense to have it as a global variable. Instead, it should defined in main()
after you have changed draw(...)
to accept it as parameter (maybe that's what list
was supposed to do in the first place?) and passed when calling draw(...)
from main()
.
Top-level script environment
In order to show what part of the file is actually supposed to be run as a script, Python defines the concept of a top-level script environment. Although the name sounds daunting at first, it's quite likely that you have seen it in someone else's Python code: if __name__ == "__main__":
This tells the Python interpreter to only run that part of code if the file is used as a script. In your case all there is to do is to do add the above right before calling the main
-function:
if __name__ == "__main__":
main()
That also helps you to avoid the issue that Python always starts a new bingo game whenever you try to import
a function from your file ;-)
edited Sep 30 at 18:12
answered Sep 29 at 22:03
AlexVAlexV
6,1572 gold badges15 silver badges40 bronze badges
6,1572 gold badges15 silver badges40 bronze badges
$begingroup$
Oh my gosh! Thank you so much for your awesome answer. I will definitely adopt all of this into my code!
$endgroup$
– Helana Brock
Sep 30 at 12:30
1
$begingroup$
random.randrange
is not an alternative torandom.sample
and neither does the documentation say so. The alternative torandom.randrange
israndom.choice(range(...))
. The thing is:random.sample
samples without replacement whilerandom.choices
and your list comprehension sample with replacement. In other words: your listcomprehension can include duplicates whilerandom.sample
can not.
$endgroup$
– Wombatz
Sep 30 at 16:53
$begingroup$
@Wombatz: I "invalidated" that part of the answer. Thanks for pointing it out.
$endgroup$
– AlexV
Sep 30 at 18:05
add a comment
|
$begingroup$
Oh my gosh! Thank you so much for your awesome answer. I will definitely adopt all of this into my code!
$endgroup$
– Helana Brock
Sep 30 at 12:30
1
$begingroup$
random.randrange
is not an alternative torandom.sample
and neither does the documentation say so. The alternative torandom.randrange
israndom.choice(range(...))
. The thing is:random.sample
samples without replacement whilerandom.choices
and your list comprehension sample with replacement. In other words: your listcomprehension can include duplicates whilerandom.sample
can not.
$endgroup$
– Wombatz
Sep 30 at 16:53
$begingroup$
@Wombatz: I "invalidated" that part of the answer. Thanks for pointing it out.
$endgroup$
– AlexV
Sep 30 at 18:05
$begingroup$
Oh my gosh! Thank you so much for your awesome answer. I will definitely adopt all of this into my code!
$endgroup$
– Helana Brock
Sep 30 at 12:30
$begingroup$
Oh my gosh! Thank you so much for your awesome answer. I will definitely adopt all of this into my code!
$endgroup$
– Helana Brock
Sep 30 at 12:30
1
1
$begingroup$
random.randrange
is not an alternative to random.sample
and neither does the documentation say so. The alternative to random.randrange
is random.choice(range(...))
. The thing is: random.sample
samples without replacement while random.choices
and your list comprehension sample with replacement. In other words: your listcomprehension can include duplicates while random.sample
can not.$endgroup$
– Wombatz
Sep 30 at 16:53
$begingroup$
random.randrange
is not an alternative to random.sample
and neither does the documentation say so. The alternative to random.randrange
is random.choice(range(...))
. The thing is: random.sample
samples without replacement while random.choices
and your list comprehension sample with replacement. In other words: your listcomprehension can include duplicates while random.sample
can not.$endgroup$
– Wombatz
Sep 30 at 16:53
$begingroup$
@Wombatz: I "invalidated" that part of the answer. Thanks for pointing it out.
$endgroup$
– AlexV
Sep 30 at 18:05
$begingroup$
@Wombatz: I "invalidated" that part of the answer. Thanks for pointing it out.
$endgroup$
– AlexV
Sep 30 at 18:05
add a comment
|
$begingroup$
Initializing your bingo card
You don't need to initialize your dictionary with lists, since random.sample
returns a list by default. Just iterate through the string "BINGO"
to set your keys. Also, the if
check doesn't need to happen, you know that you will be replacing the 3rd element of the list at the key 'N'
every time, so just have that be an instruction at the end:
def generate_card():
"""
Generates a bingo card and stores the numbers in a dictionary.
"""
# just start with a plain dictionary, random.sample
# returns a new list, so no need to pre-allocate lists
# you won't be using in the future
card =
# min and max are builtin functions, so use
# underscores to avoid name shadowing
_min = 1
_max = 15
for letter in 'BINGO':
card[letter] = random.sample(range(_min, _max), 5)
_min += 15
_max += 15
# You know which letter this needs to be, no need for the if block
card["N"][2] = "X" # free space!
return card
Checking the win condition
To check over the diagonals, you can use zip
for your keys and the indices to validate. For horizontals iterate over card.values()
and check all(item == 'X' for item in row)
. For columns, you can zip together the rows using argument unpacking.
By iterating over a dictionary, it returns the keys by default, which is why enumerate(dict)
is the desired structure here. To check the opposite direction, zip(reversed(range(5)), card)
will give you (idx, key)
pairs in opposite order in a pythonic way, since dict
cannot be reversed:
# down to the right
# iterating over the dictionary yields the keys
if all(card[k][idx]=='X' for idx, key in enumerate(card)):
print('win!')
return True
# up to the right
elif all(card[k][idx]=='X' for idx, key in zip(reversed(range(5)), card)):
print('win!')
return True
# horizontal condition
for row in card.values():
if all(item=='X' for item in row):
return True
# vertical condition
for column in zip(*card.values()):
if all(item=='X' for item in column):
return True
To show how this works:
import random
d = k: random.sample(range(10), 3) for k in 'abc'
# 'a': [5, 3, 7], 'b': [1, 8, 7], 'c': [4, 5, 8]
# diagonal
for idx, key in enumerate(d):
print(key, d[key][idx])
a 5
b 8
c 8
# opposite diagonal
for idx, key in zip(reversed(range(3)), d):
print(key, d[key][idx])
a 7
b 8
c 4
# rows
for row in d.values():
print(row)
[5, 3, 7]
[1, 8, 7]
[4, 5, 8]
for col in zip(*d.values()):
print(col)
(5, 1, 4)
(3, 8, 5)
(7, 7, 8)
Also, the return True
on all of the conditions means that the function will stop once a winning condition is hit, rather than going through all of the conditions when you really don't need to. This is called short-circuiting, meaning that you only evaluate up to the desired condition.
Looking at this a bit further, I'd probably refactor the row and column check into a different function:
def check_line(values):
"""
values is an iterable over either rows or columns
of the card. Should be iterable(str)
returns boolean
"""
for line in values:
if all(val=='X' for val in values):
return True
# the last two loops then look like:
elif check_line(card.values()):
return True
elif check_line(zip(*card.values())):
return True
return False
Class?
The fact that you're passing card
around to a lot of functions says (to me) that you could probably use a class here. The generate_card
method can be done on __init__
, and everything else takes a self
rather than a card
:
class BingoCard:
def __init__(self, _min=1, _max=15):
"""
Generates a bingo card and stores the numbers in a dictionary.
_min and _max are integers that default to 1 and 15
if a default game is desired
"""
# just start with a plain dictionary, random.sample
# returns a new list, so no need to pre-allocate lists
# you won't be using in the future
self.card =
for letter in 'BINGO':
self.card[letter] = random.sample(range(_min, _max), 5)
_min += 15
_max += 15
# You know which letter this needs to be, no need for the if block
self.card["N"][2] = "X" # free space!
# __init__ doesn't return anything
# this makes your card printable
# and requires you return a string
def __str__(self):
return 'n'.join('t'.join((letter, *map(str, values))) for letter, values in self.card.items())
def draw(self, random_draw_list):
"""
Pops a number off a list of random numbers. Using the pop method ensures no duplicate
numbers will be drawn.
Arguments:
self (dictionary): The card to to check for the number that was drawn.
list (list): The list of random numbers to be drawn from.
"""
number_drawn = random_draw_list.pop()
for letter in self.card:
# don't track an index here, use enumerate
for i, number in enumerate(self.card[letter]):
if number == number_drawn:
self.card[letter][i] = "X"
return number_drawn
def check_win(self):
# down to the right
if all(self.card[k][idx]=='X' for idx, key in enumerate(self.card)):
print('win!')
return True
# up to the right
elif all(self.card[k][idx]=='X' for idx, key in zip(reversed(range(5)), self.card)):
print('win!')
return True
# horizontal condition
elif self.check_line(self.card.values()):
return True
# vertical condition
elif self.check_line(zip(*self.card.values())):
return True
return False
@staticmethod
def check_line(values):
"""
values is an iterable over either rows or columns
of the card. Should be iterable(str)
returns boolean
"""
for line in values:
if all(val=='X' for val in values):
return True
# then, your bingo card is an instance of this class
card = BingoCard()
card.check_win()
# False
The __str__
function is outlined in the python data model, but the gist of it is that __str__
allows you to redefine what the informal string representation of an object looks like.
random_draw_list
To make this work, I'd add it as a condition on your while
loop. You pop
elements off of it, but what happens when there's nothing else to pop
? You will get an IndexError
for attempting to pop
from an empty list. To make the game re-playable, I'd have the user choose to re-play, and generate that new list on each play, just as you re-create the bingo card on each play:
def generate_random_list():
return random.sample(range(1, 76), 75)
def main():
card = BingoCard()
# generate the draw list here, on each play of the game
# that way if you run out of balls, you can handle that by restarting
# the game
random_draw_list = generate_random_list()
print(f"This is your card:n card")
# you don't need the user input here, I think it might
# be better to include it elsewhere, before generating the
# card. Instead, keep going while there are numbers to draw
while random_draw_list:
number_drawn = card.draw(random_draw_list)
# this variable is better named balls_drawn
balls_drawn += 1
print(f"You drew number_drawn")
print(f"Total balls drawn: balls_drawn")
if check_win(card):
print(f"You won after drawing balls_drawn balls!")
break
# if you were to check for user input during the game,
# it would make more sense to do that after you've at least
# checked one round
keep_playing = input("Keep playing? (y/n)").strip().lower()
if keep_playing == 'n':
print("Ok, thanks for playing")
break
else:
print("Sorry, there are no more balls to draw")
# This is where you prompt the user to play:
if __name__ == "__main__":
while True:
play = input("Would you like to play bingo? (y/n)")
if play.strip().lower() == 'y':
main()
else:
break
$endgroup$
$begingroup$
Amazing answer! I like the class idea :) I will definitely refactor that and look at what you have here when I improve my code!
$endgroup$
– Helana Brock
Sep 30 at 12:31
$begingroup$
@AlexV fair point on the class name. In the comments on that code snippet, I do explain why_min
and_max
were changed :)
$endgroup$
– C.Nivs
Sep 30 at 13:06
$begingroup$
@HelanaBrock quick note, I changed the diagonal check condition sincedict
is not reversible
$endgroup$
– C.Nivs
Sep 30 at 13:55
add a comment
|
$begingroup$
Initializing your bingo card
You don't need to initialize your dictionary with lists, since random.sample
returns a list by default. Just iterate through the string "BINGO"
to set your keys. Also, the if
check doesn't need to happen, you know that you will be replacing the 3rd element of the list at the key 'N'
every time, so just have that be an instruction at the end:
def generate_card():
"""
Generates a bingo card and stores the numbers in a dictionary.
"""
# just start with a plain dictionary, random.sample
# returns a new list, so no need to pre-allocate lists
# you won't be using in the future
card =
# min and max are builtin functions, so use
# underscores to avoid name shadowing
_min = 1
_max = 15
for letter in 'BINGO':
card[letter] = random.sample(range(_min, _max), 5)
_min += 15
_max += 15
# You know which letter this needs to be, no need for the if block
card["N"][2] = "X" # free space!
return card
Checking the win condition
To check over the diagonals, you can use zip
for your keys and the indices to validate. For horizontals iterate over card.values()
and check all(item == 'X' for item in row)
. For columns, you can zip together the rows using argument unpacking.
By iterating over a dictionary, it returns the keys by default, which is why enumerate(dict)
is the desired structure here. To check the opposite direction, zip(reversed(range(5)), card)
will give you (idx, key)
pairs in opposite order in a pythonic way, since dict
cannot be reversed:
# down to the right
# iterating over the dictionary yields the keys
if all(card[k][idx]=='X' for idx, key in enumerate(card)):
print('win!')
return True
# up to the right
elif all(card[k][idx]=='X' for idx, key in zip(reversed(range(5)), card)):
print('win!')
return True
# horizontal condition
for row in card.values():
if all(item=='X' for item in row):
return True
# vertical condition
for column in zip(*card.values()):
if all(item=='X' for item in column):
return True
To show how this works:
import random
d = k: random.sample(range(10), 3) for k in 'abc'
# 'a': [5, 3, 7], 'b': [1, 8, 7], 'c': [4, 5, 8]
# diagonal
for idx, key in enumerate(d):
print(key, d[key][idx])
a 5
b 8
c 8
# opposite diagonal
for idx, key in zip(reversed(range(3)), d):
print(key, d[key][idx])
a 7
b 8
c 4
# rows
for row in d.values():
print(row)
[5, 3, 7]
[1, 8, 7]
[4, 5, 8]
for col in zip(*d.values()):
print(col)
(5, 1, 4)
(3, 8, 5)
(7, 7, 8)
Also, the return True
on all of the conditions means that the function will stop once a winning condition is hit, rather than going through all of the conditions when you really don't need to. This is called short-circuiting, meaning that you only evaluate up to the desired condition.
Looking at this a bit further, I'd probably refactor the row and column check into a different function:
def check_line(values):
"""
values is an iterable over either rows or columns
of the card. Should be iterable(str)
returns boolean
"""
for line in values:
if all(val=='X' for val in values):
return True
# the last two loops then look like:
elif check_line(card.values()):
return True
elif check_line(zip(*card.values())):
return True
return False
Class?
The fact that you're passing card
around to a lot of functions says (to me) that you could probably use a class here. The generate_card
method can be done on __init__
, and everything else takes a self
rather than a card
:
class BingoCard:
def __init__(self, _min=1, _max=15):
"""
Generates a bingo card and stores the numbers in a dictionary.
_min and _max are integers that default to 1 and 15
if a default game is desired
"""
# just start with a plain dictionary, random.sample
# returns a new list, so no need to pre-allocate lists
# you won't be using in the future
self.card =
for letter in 'BINGO':
self.card[letter] = random.sample(range(_min, _max), 5)
_min += 15
_max += 15
# You know which letter this needs to be, no need for the if block
self.card["N"][2] = "X" # free space!
# __init__ doesn't return anything
# this makes your card printable
# and requires you return a string
def __str__(self):
return 'n'.join('t'.join((letter, *map(str, values))) for letter, values in self.card.items())
def draw(self, random_draw_list):
"""
Pops a number off a list of random numbers. Using the pop method ensures no duplicate
numbers will be drawn.
Arguments:
self (dictionary): The card to to check for the number that was drawn.
list (list): The list of random numbers to be drawn from.
"""
number_drawn = random_draw_list.pop()
for letter in self.card:
# don't track an index here, use enumerate
for i, number in enumerate(self.card[letter]):
if number == number_drawn:
self.card[letter][i] = "X"
return number_drawn
def check_win(self):
# down to the right
if all(self.card[k][idx]=='X' for idx, key in enumerate(self.card)):
print('win!')
return True
# up to the right
elif all(self.card[k][idx]=='X' for idx, key in zip(reversed(range(5)), self.card)):
print('win!')
return True
# horizontal condition
elif self.check_line(self.card.values()):
return True
# vertical condition
elif self.check_line(zip(*self.card.values())):
return True
return False
@staticmethod
def check_line(values):
"""
values is an iterable over either rows or columns
of the card. Should be iterable(str)
returns boolean
"""
for line in values:
if all(val=='X' for val in values):
return True
# then, your bingo card is an instance of this class
card = BingoCard()
card.check_win()
# False
The __str__
function is outlined in the python data model, but the gist of it is that __str__
allows you to redefine what the informal string representation of an object looks like.
random_draw_list
To make this work, I'd add it as a condition on your while
loop. You pop
elements off of it, but what happens when there's nothing else to pop
? You will get an IndexError
for attempting to pop
from an empty list. To make the game re-playable, I'd have the user choose to re-play, and generate that new list on each play, just as you re-create the bingo card on each play:
def generate_random_list():
return random.sample(range(1, 76), 75)
def main():
card = BingoCard()
# generate the draw list here, on each play of the game
# that way if you run out of balls, you can handle that by restarting
# the game
random_draw_list = generate_random_list()
print(f"This is your card:n card")
# you don't need the user input here, I think it might
# be better to include it elsewhere, before generating the
# card. Instead, keep going while there are numbers to draw
while random_draw_list:
number_drawn = card.draw(random_draw_list)
# this variable is better named balls_drawn
balls_drawn += 1
print(f"You drew number_drawn")
print(f"Total balls drawn: balls_drawn")
if check_win(card):
print(f"You won after drawing balls_drawn balls!")
break
# if you were to check for user input during the game,
# it would make more sense to do that after you've at least
# checked one round
keep_playing = input("Keep playing? (y/n)").strip().lower()
if keep_playing == 'n':
print("Ok, thanks for playing")
break
else:
print("Sorry, there are no more balls to draw")
# This is where you prompt the user to play:
if __name__ == "__main__":
while True:
play = input("Would you like to play bingo? (y/n)")
if play.strip().lower() == 'y':
main()
else:
break
$endgroup$
$begingroup$
Amazing answer! I like the class idea :) I will definitely refactor that and look at what you have here when I improve my code!
$endgroup$
– Helana Brock
Sep 30 at 12:31
$begingroup$
@AlexV fair point on the class name. In the comments on that code snippet, I do explain why_min
and_max
were changed :)
$endgroup$
– C.Nivs
Sep 30 at 13:06
$begingroup$
@HelanaBrock quick note, I changed the diagonal check condition sincedict
is not reversible
$endgroup$
– C.Nivs
Sep 30 at 13:55
add a comment
|
$begingroup$
Initializing your bingo card
You don't need to initialize your dictionary with lists, since random.sample
returns a list by default. Just iterate through the string "BINGO"
to set your keys. Also, the if
check doesn't need to happen, you know that you will be replacing the 3rd element of the list at the key 'N'
every time, so just have that be an instruction at the end:
def generate_card():
"""
Generates a bingo card and stores the numbers in a dictionary.
"""
# just start with a plain dictionary, random.sample
# returns a new list, so no need to pre-allocate lists
# you won't be using in the future
card =
# min and max are builtin functions, so use
# underscores to avoid name shadowing
_min = 1
_max = 15
for letter in 'BINGO':
card[letter] = random.sample(range(_min, _max), 5)
_min += 15
_max += 15
# You know which letter this needs to be, no need for the if block
card["N"][2] = "X" # free space!
return card
Checking the win condition
To check over the diagonals, you can use zip
for your keys and the indices to validate. For horizontals iterate over card.values()
and check all(item == 'X' for item in row)
. For columns, you can zip together the rows using argument unpacking.
By iterating over a dictionary, it returns the keys by default, which is why enumerate(dict)
is the desired structure here. To check the opposite direction, zip(reversed(range(5)), card)
will give you (idx, key)
pairs in opposite order in a pythonic way, since dict
cannot be reversed:
# down to the right
# iterating over the dictionary yields the keys
if all(card[k][idx]=='X' for idx, key in enumerate(card)):
print('win!')
return True
# up to the right
elif all(card[k][idx]=='X' for idx, key in zip(reversed(range(5)), card)):
print('win!')
return True
# horizontal condition
for row in card.values():
if all(item=='X' for item in row):
return True
# vertical condition
for column in zip(*card.values()):
if all(item=='X' for item in column):
return True
To show how this works:
import random
d = k: random.sample(range(10), 3) for k in 'abc'
# 'a': [5, 3, 7], 'b': [1, 8, 7], 'c': [4, 5, 8]
# diagonal
for idx, key in enumerate(d):
print(key, d[key][idx])
a 5
b 8
c 8
# opposite diagonal
for idx, key in zip(reversed(range(3)), d):
print(key, d[key][idx])
a 7
b 8
c 4
# rows
for row in d.values():
print(row)
[5, 3, 7]
[1, 8, 7]
[4, 5, 8]
for col in zip(*d.values()):
print(col)
(5, 1, 4)
(3, 8, 5)
(7, 7, 8)
Also, the return True
on all of the conditions means that the function will stop once a winning condition is hit, rather than going through all of the conditions when you really don't need to. This is called short-circuiting, meaning that you only evaluate up to the desired condition.
Looking at this a bit further, I'd probably refactor the row and column check into a different function:
def check_line(values):
"""
values is an iterable over either rows or columns
of the card. Should be iterable(str)
returns boolean
"""
for line in values:
if all(val=='X' for val in values):
return True
# the last two loops then look like:
elif check_line(card.values()):
return True
elif check_line(zip(*card.values())):
return True
return False
Class?
The fact that you're passing card
around to a lot of functions says (to me) that you could probably use a class here. The generate_card
method can be done on __init__
, and everything else takes a self
rather than a card
:
class BingoCard:
def __init__(self, _min=1, _max=15):
"""
Generates a bingo card and stores the numbers in a dictionary.
_min and _max are integers that default to 1 and 15
if a default game is desired
"""
# just start with a plain dictionary, random.sample
# returns a new list, so no need to pre-allocate lists
# you won't be using in the future
self.card =
for letter in 'BINGO':
self.card[letter] = random.sample(range(_min, _max), 5)
_min += 15
_max += 15
# You know which letter this needs to be, no need for the if block
self.card["N"][2] = "X" # free space!
# __init__ doesn't return anything
# this makes your card printable
# and requires you return a string
def __str__(self):
return 'n'.join('t'.join((letter, *map(str, values))) for letter, values in self.card.items())
def draw(self, random_draw_list):
"""
Pops a number off a list of random numbers. Using the pop method ensures no duplicate
numbers will be drawn.
Arguments:
self (dictionary): The card to to check for the number that was drawn.
list (list): The list of random numbers to be drawn from.
"""
number_drawn = random_draw_list.pop()
for letter in self.card:
# don't track an index here, use enumerate
for i, number in enumerate(self.card[letter]):
if number == number_drawn:
self.card[letter][i] = "X"
return number_drawn
def check_win(self):
# down to the right
if all(self.card[k][idx]=='X' for idx, key in enumerate(self.card)):
print('win!')
return True
# up to the right
elif all(self.card[k][idx]=='X' for idx, key in zip(reversed(range(5)), self.card)):
print('win!')
return True
# horizontal condition
elif self.check_line(self.card.values()):
return True
# vertical condition
elif self.check_line(zip(*self.card.values())):
return True
return False
@staticmethod
def check_line(values):
"""
values is an iterable over either rows or columns
of the card. Should be iterable(str)
returns boolean
"""
for line in values:
if all(val=='X' for val in values):
return True
# then, your bingo card is an instance of this class
card = BingoCard()
card.check_win()
# False
The __str__
function is outlined in the python data model, but the gist of it is that __str__
allows you to redefine what the informal string representation of an object looks like.
random_draw_list
To make this work, I'd add it as a condition on your while
loop. You pop
elements off of it, but what happens when there's nothing else to pop
? You will get an IndexError
for attempting to pop
from an empty list. To make the game re-playable, I'd have the user choose to re-play, and generate that new list on each play, just as you re-create the bingo card on each play:
def generate_random_list():
return random.sample(range(1, 76), 75)
def main():
card = BingoCard()
# generate the draw list here, on each play of the game
# that way if you run out of balls, you can handle that by restarting
# the game
random_draw_list = generate_random_list()
print(f"This is your card:n card")
# you don't need the user input here, I think it might
# be better to include it elsewhere, before generating the
# card. Instead, keep going while there are numbers to draw
while random_draw_list:
number_drawn = card.draw(random_draw_list)
# this variable is better named balls_drawn
balls_drawn += 1
print(f"You drew number_drawn")
print(f"Total balls drawn: balls_drawn")
if check_win(card):
print(f"You won after drawing balls_drawn balls!")
break
# if you were to check for user input during the game,
# it would make more sense to do that after you've at least
# checked one round
keep_playing = input("Keep playing? (y/n)").strip().lower()
if keep_playing == 'n':
print("Ok, thanks for playing")
break
else:
print("Sorry, there are no more balls to draw")
# This is where you prompt the user to play:
if __name__ == "__main__":
while True:
play = input("Would you like to play bingo? (y/n)")
if play.strip().lower() == 'y':
main()
else:
break
$endgroup$
Initializing your bingo card
You don't need to initialize your dictionary with lists, since random.sample
returns a list by default. Just iterate through the string "BINGO"
to set your keys. Also, the if
check doesn't need to happen, you know that you will be replacing the 3rd element of the list at the key 'N'
every time, so just have that be an instruction at the end:
def generate_card():
"""
Generates a bingo card and stores the numbers in a dictionary.
"""
# just start with a plain dictionary, random.sample
# returns a new list, so no need to pre-allocate lists
# you won't be using in the future
card =
# min and max are builtin functions, so use
# underscores to avoid name shadowing
_min = 1
_max = 15
for letter in 'BINGO':
card[letter] = random.sample(range(_min, _max), 5)
_min += 15
_max += 15
# You know which letter this needs to be, no need for the if block
card["N"][2] = "X" # free space!
return card
Checking the win condition
To check over the diagonals, you can use zip
for your keys and the indices to validate. For horizontals iterate over card.values()
and check all(item == 'X' for item in row)
. For columns, you can zip together the rows using argument unpacking.
By iterating over a dictionary, it returns the keys by default, which is why enumerate(dict)
is the desired structure here. To check the opposite direction, zip(reversed(range(5)), card)
will give you (idx, key)
pairs in opposite order in a pythonic way, since dict
cannot be reversed:
# down to the right
# iterating over the dictionary yields the keys
if all(card[k][idx]=='X' for idx, key in enumerate(card)):
print('win!')
return True
# up to the right
elif all(card[k][idx]=='X' for idx, key in zip(reversed(range(5)), card)):
print('win!')
return True
# horizontal condition
for row in card.values():
if all(item=='X' for item in row):
return True
# vertical condition
for column in zip(*card.values()):
if all(item=='X' for item in column):
return True
To show how this works:
import random
d = k: random.sample(range(10), 3) for k in 'abc'
# 'a': [5, 3, 7], 'b': [1, 8, 7], 'c': [4, 5, 8]
# diagonal
for idx, key in enumerate(d):
print(key, d[key][idx])
a 5
b 8
c 8
# opposite diagonal
for idx, key in zip(reversed(range(3)), d):
print(key, d[key][idx])
a 7
b 8
c 4
# rows
for row in d.values():
print(row)
[5, 3, 7]
[1, 8, 7]
[4, 5, 8]
for col in zip(*d.values()):
print(col)
(5, 1, 4)
(3, 8, 5)
(7, 7, 8)
Also, the return True
on all of the conditions means that the function will stop once a winning condition is hit, rather than going through all of the conditions when you really don't need to. This is called short-circuiting, meaning that you only evaluate up to the desired condition.
Looking at this a bit further, I'd probably refactor the row and column check into a different function:
def check_line(values):
"""
values is an iterable over either rows or columns
of the card. Should be iterable(str)
returns boolean
"""
for line in values:
if all(val=='X' for val in values):
return True
# the last two loops then look like:
elif check_line(card.values()):
return True
elif check_line(zip(*card.values())):
return True
return False
Class?
The fact that you're passing card
around to a lot of functions says (to me) that you could probably use a class here. The generate_card
method can be done on __init__
, and everything else takes a self
rather than a card
:
class BingoCard:
def __init__(self, _min=1, _max=15):
"""
Generates a bingo card and stores the numbers in a dictionary.
_min and _max are integers that default to 1 and 15
if a default game is desired
"""
# just start with a plain dictionary, random.sample
# returns a new list, so no need to pre-allocate lists
# you won't be using in the future
self.card =
for letter in 'BINGO':
self.card[letter] = random.sample(range(_min, _max), 5)
_min += 15
_max += 15
# You know which letter this needs to be, no need for the if block
self.card["N"][2] = "X" # free space!
# __init__ doesn't return anything
# this makes your card printable
# and requires you return a string
def __str__(self):
return 'n'.join('t'.join((letter, *map(str, values))) for letter, values in self.card.items())
def draw(self, random_draw_list):
"""
Pops a number off a list of random numbers. Using the pop method ensures no duplicate
numbers will be drawn.
Arguments:
self (dictionary): The card to to check for the number that was drawn.
list (list): The list of random numbers to be drawn from.
"""
number_drawn = random_draw_list.pop()
for letter in self.card:
# don't track an index here, use enumerate
for i, number in enumerate(self.card[letter]):
if number == number_drawn:
self.card[letter][i] = "X"
return number_drawn
def check_win(self):
# down to the right
if all(self.card[k][idx]=='X' for idx, key in enumerate(self.card)):
print('win!')
return True
# up to the right
elif all(self.card[k][idx]=='X' for idx, key in zip(reversed(range(5)), self.card)):
print('win!')
return True
# horizontal condition
elif self.check_line(self.card.values()):
return True
# vertical condition
elif self.check_line(zip(*self.card.values())):
return True
return False
@staticmethod
def check_line(values):
"""
values is an iterable over either rows or columns
of the card. Should be iterable(str)
returns boolean
"""
for line in values:
if all(val=='X' for val in values):
return True
# then, your bingo card is an instance of this class
card = BingoCard()
card.check_win()
# False
The __str__
function is outlined in the python data model, but the gist of it is that __str__
allows you to redefine what the informal string representation of an object looks like.
random_draw_list
To make this work, I'd add it as a condition on your while
loop. You pop
elements off of it, but what happens when there's nothing else to pop
? You will get an IndexError
for attempting to pop
from an empty list. To make the game re-playable, I'd have the user choose to re-play, and generate that new list on each play, just as you re-create the bingo card on each play:
def generate_random_list():
return random.sample(range(1, 76), 75)
def main():
card = BingoCard()
# generate the draw list here, on each play of the game
# that way if you run out of balls, you can handle that by restarting
# the game
random_draw_list = generate_random_list()
print(f"This is your card:n card")
# you don't need the user input here, I think it might
# be better to include it elsewhere, before generating the
# card. Instead, keep going while there are numbers to draw
while random_draw_list:
number_drawn = card.draw(random_draw_list)
# this variable is better named balls_drawn
balls_drawn += 1
print(f"You drew number_drawn")
print(f"Total balls drawn: balls_drawn")
if check_win(card):
print(f"You won after drawing balls_drawn balls!")
break
# if you were to check for user input during the game,
# it would make more sense to do that after you've at least
# checked one round
keep_playing = input("Keep playing? (y/n)").strip().lower()
if keep_playing == 'n':
print("Ok, thanks for playing")
break
else:
print("Sorry, there are no more balls to draw")
# This is where you prompt the user to play:
if __name__ == "__main__":
while True:
play = input("Would you like to play bingo? (y/n)")
if play.strip().lower() == 'y':
main()
else:
break
edited Sep 30 at 14:53
answered Sep 29 at 22:17
C.NivsC.Nivs
8786 silver badges17 bronze badges
8786 silver badges17 bronze badges
$begingroup$
Amazing answer! I like the class idea :) I will definitely refactor that and look at what you have here when I improve my code!
$endgroup$
– Helana Brock
Sep 30 at 12:31
$begingroup$
@AlexV fair point on the class name. In the comments on that code snippet, I do explain why_min
and_max
were changed :)
$endgroup$
– C.Nivs
Sep 30 at 13:06
$begingroup$
@HelanaBrock quick note, I changed the diagonal check condition sincedict
is not reversible
$endgroup$
– C.Nivs
Sep 30 at 13:55
add a comment
|
$begingroup$
Amazing answer! I like the class idea :) I will definitely refactor that and look at what you have here when I improve my code!
$endgroup$
– Helana Brock
Sep 30 at 12:31
$begingroup$
@AlexV fair point on the class name. In the comments on that code snippet, I do explain why_min
and_max
were changed :)
$endgroup$
– C.Nivs
Sep 30 at 13:06
$begingroup$
@HelanaBrock quick note, I changed the diagonal check condition sincedict
is not reversible
$endgroup$
– C.Nivs
Sep 30 at 13:55
$begingroup$
Amazing answer! I like the class idea :) I will definitely refactor that and look at what you have here when I improve my code!
$endgroup$
– Helana Brock
Sep 30 at 12:31
$begingroup$
Amazing answer! I like the class idea :) I will definitely refactor that and look at what you have here when I improve my code!
$endgroup$
– Helana Brock
Sep 30 at 12:31
$begingroup$
@AlexV fair point on the class name. In the comments on that code snippet, I do explain why
_min
and _max
were changed :)$endgroup$
– C.Nivs
Sep 30 at 13:06
$begingroup$
@AlexV fair point on the class name. In the comments on that code snippet, I do explain why
_min
and _max
were changed :)$endgroup$
– C.Nivs
Sep 30 at 13:06
$begingroup$
@HelanaBrock quick note, I changed the diagonal check condition since
dict
is not reversible$endgroup$
– C.Nivs
Sep 30 at 13:55
$begingroup$
@HelanaBrock quick note, I changed the diagonal check condition since
dict
is not reversible$endgroup$
– C.Nivs
Sep 30 at 13:55
add a comment
|
$begingroup$
Some low-hanging fruit:
if win == True
can be written as
if win:
Know that print
delimits by a newline by default; you don't need to explicitly add n
unless you want two newlines.
You can use classes to make card
a class on its own.
$endgroup$
$begingroup$
Thank you so much! I will change that, I didnt think about it. Also yes I wanted to two newlines haha
$endgroup$
– Helana Brock
Sep 30 at 12:29
add a comment
|
$begingroup$
Some low-hanging fruit:
if win == True
can be written as
if win:
Know that print
delimits by a newline by default; you don't need to explicitly add n
unless you want two newlines.
You can use classes to make card
a class on its own.
$endgroup$
$begingroup$
Thank you so much! I will change that, I didnt think about it. Also yes I wanted to two newlines haha
$endgroup$
– Helana Brock
Sep 30 at 12:29
add a comment
|
$begingroup$
Some low-hanging fruit:
if win == True
can be written as
if win:
Know that print
delimits by a newline by default; you don't need to explicitly add n
unless you want two newlines.
You can use classes to make card
a class on its own.
$endgroup$
Some low-hanging fruit:
if win == True
can be written as
if win:
Know that print
delimits by a newline by default; you don't need to explicitly add n
unless you want two newlines.
You can use classes to make card
a class on its own.
answered Sep 29 at 20:47
nz_21nz_21
7452 silver badges13 bronze badges
7452 silver badges13 bronze badges
$begingroup$
Thank you so much! I will change that, I didnt think about it. Also yes I wanted to two newlines haha
$endgroup$
– Helana Brock
Sep 30 at 12:29
add a comment
|
$begingroup$
Thank you so much! I will change that, I didnt think about it. Also yes I wanted to two newlines haha
$endgroup$
– Helana Brock
Sep 30 at 12:29
$begingroup$
Thank you so much! I will change that, I didnt think about it. Also yes I wanted to two newlines haha
$endgroup$
– Helana Brock
Sep 30 at 12:29
$begingroup$
Thank you so much! I will change that, I didnt think about it. Also yes I wanted to two newlines haha
$endgroup$
– Helana Brock
Sep 30 at 12:29
add a comment
|
$begingroup$
Nice job. Here's two comments to add to what other's have said.
range() excludes the stop point
The built-in function range(start, stop, step)
does not include stop
in the items it returns. So in generate_card()
:
min_ = 1
max_ = 15
...
random.sample(range(min_, max_), 5)
only selects from the numbers 1, 2, 3, ..., 14. max_
should be 16. I sometimes write code with an explicit +1
like this:
random.sample(range(min_, max_+1), 5)
to remind myself that I intended to include max_
in the range.
Separation of concerns
When a function in a program does multiple unrelated things, it may make the program more difficult to debug, modify, or enhance. For example, draw()
gets the next numbered ball and adds it to the BINGO card. If a the player could have multiple bingo cards, each card would be drawing it's own random balls. Clearly, that wouldn't work. It would be better if the ball was drawn separately and then checked against each card.
$endgroup$
$begingroup$
I like the separation of concerns point a lot, +1. To drive that home, I'd argue thatdrawing
is a function of the game, not the card, so it should not be implemented at the card level. Instead, draw should take anumber
rather than anumber_list
as an argument, and that should be checked against the card
$endgroup$
– C.Nivs
Sep 30 at 16:00
add a comment
|
$begingroup$
Nice job. Here's two comments to add to what other's have said.
range() excludes the stop point
The built-in function range(start, stop, step)
does not include stop
in the items it returns. So in generate_card()
:
min_ = 1
max_ = 15
...
random.sample(range(min_, max_), 5)
only selects from the numbers 1, 2, 3, ..., 14. max_
should be 16. I sometimes write code with an explicit +1
like this:
random.sample(range(min_, max_+1), 5)
to remind myself that I intended to include max_
in the range.
Separation of concerns
When a function in a program does multiple unrelated things, it may make the program more difficult to debug, modify, or enhance. For example, draw()
gets the next numbered ball and adds it to the BINGO card. If a the player could have multiple bingo cards, each card would be drawing it's own random balls. Clearly, that wouldn't work. It would be better if the ball was drawn separately and then checked against each card.
$endgroup$
$begingroup$
I like the separation of concerns point a lot, +1. To drive that home, I'd argue thatdrawing
is a function of the game, not the card, so it should not be implemented at the card level. Instead, draw should take anumber
rather than anumber_list
as an argument, and that should be checked against the card
$endgroup$
– C.Nivs
Sep 30 at 16:00
add a comment
|
$begingroup$
Nice job. Here's two comments to add to what other's have said.
range() excludes the stop point
The built-in function range(start, stop, step)
does not include stop
in the items it returns. So in generate_card()
:
min_ = 1
max_ = 15
...
random.sample(range(min_, max_), 5)
only selects from the numbers 1, 2, 3, ..., 14. max_
should be 16. I sometimes write code with an explicit +1
like this:
random.sample(range(min_, max_+1), 5)
to remind myself that I intended to include max_
in the range.
Separation of concerns
When a function in a program does multiple unrelated things, it may make the program more difficult to debug, modify, or enhance. For example, draw()
gets the next numbered ball and adds it to the BINGO card. If a the player could have multiple bingo cards, each card would be drawing it's own random balls. Clearly, that wouldn't work. It would be better if the ball was drawn separately and then checked against each card.
$endgroup$
Nice job. Here's two comments to add to what other's have said.
range() excludes the stop point
The built-in function range(start, stop, step)
does not include stop
in the items it returns. So in generate_card()
:
min_ = 1
max_ = 15
...
random.sample(range(min_, max_), 5)
only selects from the numbers 1, 2, 3, ..., 14. max_
should be 16. I sometimes write code with an explicit +1
like this:
random.sample(range(min_, max_+1), 5)
to remind myself that I intended to include max_
in the range.
Separation of concerns
When a function in a program does multiple unrelated things, it may make the program more difficult to debug, modify, or enhance. For example, draw()
gets the next numbered ball and adds it to the BINGO card. If a the player could have multiple bingo cards, each card would be drawing it's own random balls. Clearly, that wouldn't work. It would be better if the ball was drawn separately and then checked against each card.
answered Sep 30 at 15:07
RootTwoRootTwo
3,5297 silver badges16 bronze badges
3,5297 silver badges16 bronze badges
$begingroup$
I like the separation of concerns point a lot, +1. To drive that home, I'd argue thatdrawing
is a function of the game, not the card, so it should not be implemented at the card level. Instead, draw should take anumber
rather than anumber_list
as an argument, and that should be checked against the card
$endgroup$
– C.Nivs
Sep 30 at 16:00
add a comment
|
$begingroup$
I like the separation of concerns point a lot, +1. To drive that home, I'd argue thatdrawing
is a function of the game, not the card, so it should not be implemented at the card level. Instead, draw should take anumber
rather than anumber_list
as an argument, and that should be checked against the card
$endgroup$
– C.Nivs
Sep 30 at 16:00
$begingroup$
I like the separation of concerns point a lot, +1. To drive that home, I'd argue that
drawing
is a function of the game, not the card, so it should not be implemented at the card level. Instead, draw should take a number
rather than a number_list
as an argument, and that should be checked against the card$endgroup$
– C.Nivs
Sep 30 at 16:00
$begingroup$
I like the separation of concerns point a lot, +1. To drive that home, I'd argue that
drawing
is a function of the game, not the card, so it should not be implemented at the card level. Instead, draw should take a number
rather than a number_list
as an argument, and that should be checked against the card$endgroup$
– C.Nivs
Sep 30 at 16:00
add a comment
|
Thanks for contributing an answer to Code Review Stack Exchange!
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
Use MathJax to format equations. MathJax reference.
To learn more, see our tips on writing great answers.
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f229872%2fpython-bingo-game-that-stores-card-in-a-dictionary%23new-answer', 'question_page');
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown