3
\$\begingroup\$

I have a simple task of creating a simple lotto game to practice python. I had some troubles with return of functions so i dropped few because of not understanding the scopes maybe.

In my next version i want to try to make it fully in functions, and in the next version i want to do it in OOP principles.

By posting this code I would like to hear maybe some suggestions where my logic was wrong and some PEP comments.

Game rules

Each player is given 15 numbers, in increasing order. This is why i use sorted() on the lists of player and computer. Also there is a common list with numbers in it from 1 to 90.

A random number is picked and removed from this list and printed to players. The player who has this number in his card crosses it out or skips its turn. This continues until either player or computer crosses out all his numbers, or for some strange reason they are out of numbers :) I think in other parts of the world it is also called BINGO but I might be wrong.

import random

number_pool = list(range(1, 91))
computer_card = random.sample(number_pool, 15)
computer_card_sorted = sorted(computer_card)
player_card = random.sample(number_pool, 15)
player_card_sorted = sorted(player_card)


def display_cards():
    print("Computer card:\n")
    print(computer_card_sorted)
    print("====================================")
    print("Player card:\n")
    print(player_card_sorted)


def lotto_choice():
    choice = random.choice(number_pool)
    number_pool.remove(choice)
    return choice


def the_game():
    while number_pool:
        choice = lotto_choice()
        print("The random lotto is: " + str(choice))
        display_cards()
        cross_number = input("Do you want to cross out a number")
        cross_number.lower()
        if cross_number == "y":
            if choice in player_card_sorted:
                player_card_sorted.remove(choice)
            elif choice in computer_card_sorted:
                computer_card_sorted.remove(choice)
        if cross_number == "n":
            if choice in computer_card_sorted:
                computer_card_sorted.remove(choice)
            else:
                continue
    else:
        if len(player_card_sorted) == 0:
            print("Congratulations Player ! You won")
        elif len(computer_card_sorted) == 0:
            print("The computer have won, too bad !")
        else:
            print("It is a tie you both ran out of numbers, very straange !")


the_game()
\$\endgroup\$
  • \$\begingroup\$ ty, yes as far as i tested it it works fine \$\endgroup\$ – Sergo Gumkadze Jul 12 '17 at 11:59
2
\$\begingroup\$

For your current purposes, the biggest problem is that you are relying on global variables exclusively. This makes it a lot tougher to understand, maintain, and debug your code. This question actually has some great answers that discuss why this can cause real problems in code:

https://softwareengineering.stackexchange.com/questions/148108/why-is-global-state-so-evil

As a specific example, this bit of code had me confused for a few minutes:

while number_pool:

Because number_pool wasn't used anywhere in your while loop, and so I had no idea why this wasn't just an infinite loop. I eventually realized that it is getting modified inside the lotto_choice method which is called within the while loop. This is pretty much exactly why you don't want to use global state: if any bit of code can modify any variable anywhere, you can quickly lose track of who modifies what and when it happens, and bugs can creep in very quickly. You also will have your own trouble if you come back to the code after a while and find that you no longer remember exactly what gets modified by what functions.

So, always make sure and keep input into functions and output out of functions clear. In the case of lotto_choice you could easily do something like this:

def lotto_choice( number_pool ):
    choice = random.choice(number_pool)
    number_pool.remove(choice)
    return ( choice, number_pool )

Then you would call it like this inside your while loop:

( choice, number_pool ) = lotto_choice( number_pool )

Now you can easily see where number_pool gets modified. Since you are just now getting started, now is the time to establish good habits. So take the time and understand how variable scope works and (especially in a learning exercise like this) make sure not to use any global state. My biggest suggestion is to fix that and then post a new code review question with your updated code.

Also, small change. You want to only execute your game if called in the 'main' context, like this:

if __name__ == '__main__':
    the_game()
\$\endgroup\$
  • \$\begingroup\$ Thanks alot for your review and advice ! I will rewrite and get back with another version \$\endgroup\$ – Sergo Gumkadze Jul 13 '17 at 5:51
1
\$\begingroup\$

Take a look on this, I think it's cleaner. I've deleted the option to choose if you want to cross number if there's a number, because obviously the player will always want to cross it if it is in his list. I also deleted some variables (sorted lists) and just call them computer_list and player_list. I hope it helps :)

import random

number_pool = [i for i in range(1, 91)]
random.shuffle(number_pool)
computer_card = sorted(random.sample(number_pool, 15))
player_card = sorted(random.sample(number_pool, 15))


def display_cards():
    print("=================================================================")
    print("= Computer card:                                                =\n")
    print(computer_card)
    print("=================================================================")
    print("= Player card:                                                  =\n")
    print(player_card)
    print("=================================================================")


def winner_found():
    if (len(computer_card) == 0 or len(player_card) == 0):
        return True
    return False


def who_won():
    if(len(player_card) == 0):
        return "Player"
    if(len(computer_card) == 0):
        return "Computer"


def wait():
    input()


def main():
    while (not winner_found()):
        print("\n########### NEW ROW ###########")
        random_choice = number_pool.pop()
        print("The random lotto is: {} ".format(random_choice))
        display_cards()
        if random_choice in player_card:
            player_card.remove(random_choice)
        if random_choice in computer_card:
            computer_card.remove(random_choice)

        wait()
    # If winner_found():
    print("There's a winner: The {}!" .format(who_won()))


if __name__ == '__main__':
    main()
\$\endgroup\$
  • \$\begingroup\$ And thanks alot to you sir :) Got some ideas to think of too, cheers ! \$\endgroup\$ – Sergo Gumkadze Jul 13 '17 at 5:51

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.