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()

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.

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 alread 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.

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 paramter (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 ;-)

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". 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:

# down to the right

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 enumerate(reversed(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 'abcd'}

# {'a': [5, 3, 7], 'b': [1, 8, 7], 'c': [4, 5, 8], 'd': [2, 4, 9]}



# rows

for row in d.values():

    print(row)



[5, 3, 7]

[1, 8, 7]

[4, 5, 8]

[2, 4, 9]



for col in zip(*d.values()):

    print(col)



(5, 1, 4, 2)

(3, 8, 5, 4)

(7, 7, 8, 9)

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.

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

    def __repr__(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 enumerate(reversed(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

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