4
\$\begingroup\$

This is a simple rock, paper scissors game I attempted as part of the Automate the Boring Stuff with Python course on Udemy. The scope of the game is for the user to enter a valid input, the system then returns the result and the current score. If the user wishes to quit the game he/she enters 'q'

So far I have covered the following topics:

  1. Flow Control (Includes If, Elif, Else statements & While/For loops)
  2. Functions (Includes pythons built in functions, writings ones own function & local & global scopes)
  3. Lists

Considering what I have studied so far, I would like to get feedback on the following code:

import sys, random #Importing sys & random modules
print ('ROCK, PAPER, SCISSORS Game')
wins, losses, draws = 0, 0, 0

def turn_generation():
    global turn #Making "turn" a global variable
    global computer_turn #Making "computer_turn" a global variable
    turn = random.randint(1,3)
    if turn == 1:
        computer_turn = 'r'
    elif turn == 2:
        computer_turn = 'p'
    elif turn == 3:
        computer_turn = 's'

while True:                           #Main loop of the game which asks the player to enter an input
    print(str(wins) + '  Wins  || ' + str(losses) + '  Losses  ||  ' + str(draws) + '  Draws')
    try:
        players_turn = input('Enter your move: (r)ock (p)aper (s)cissors or (q)uit ')
        turn_generation()
        if players_turn == computer_turn:  #In case both the moves by computer and player are same, this will execute.
            print('It is a draw!')
            draws += 1

    
    elif players_turn == 'r' and computer_turn == 's':
        print('Player wins!')
        wins += 1
    elif players_turn == 'p' and computer_turn == 'r':
        print('Player wins!')
        wins += 1
    elif players_turn == 's' and computer_turn == 'p':
        print('Player wins!')
        wins += 1

    
    elif players_turn == 'p' and computer_turn == 's': 
        print('Computer wins!')
        losses += 1
    elif players_turn == 's' and computer_turn == 'r':
        print('Computer wins!')
        losses += 1
    elif players_turn == 'r' and computer_turn == 'p':
        print('Computer wins!')
        losses += 1

    
    elif players_turn == 'q':
        sys.exit() #Ending program if player enters "q"
except:
    print('Incorrect value entered')

I would like to know if this program can be optimized in anyway within the topics I have covered.

\$\endgroup\$

2 Answers 2

5
\$\begingroup\$

Don't use bare except statements. Under nearly all conditions, you should specify some kind of Exception; otherwise, even low-level exceptions (like SystemExit or KeyboardInterrupt) will be caught.

try:
    ...
    elif players_turn == 'q':
        sys.exit()
except Exception:
    # Just catch garden-variety exceptions, not low-level.
    ...

Global variables are almost never needed. And they are also almost always a bad idea. Organize your programs around functions that take arguments and return data. There are many benefits to this approach.

Wrap try-except around the narrowest scope possible. Currently, a large portion of your code is wrapped in a try-except structure. That causes many potential problems, but the immediate practical one is the difficulty of writing and debugging code under those conditions: instead of errors occurring loudly and verbosely, they will be cloaked by whatever you do or don't do in the except block. The main reason for exception handling in this program is to deal with the user's input. In that case, put that one step in a function:

ROCK = 'rock'
PAPER = 'paper'
SCISSORS = 'scissors'

CHOICES = {
    'r': ROCK,
    'p': PAPER,
    's': SCISSORS,
    'q': None,
}

def get_player_choice():
    while True:
        try:
            c = input('Enter your move: (r)ock (p)aper (s)cissors or (q)uit ')
            return CHOICES[c]
        except KeyError:
            # When possible declare the specific errors you intend to catch.
            print('Incorrect value entered')

The random module has many features. One of them is selecting a random element from a sequence.

def get_computer_choice():
    return random.choice([ROCK, PAPER, SCISSORS])

If your code is super repetitive, you probably need some data structures. Most of your logic consists of routine, repetitive, typo-prone code checking all combinations of rock, paper, scissors. But you could list all of those combinations in one tidy spot, and then use that data structure to drive most of the program's logic. You can also use a data structure to keep a tally of wins, draws, and losses. If you haven't learned about dicts yet, now is the time to start.

OUTCOMES = {
    (ROCK, ROCK): 'Draw',
    (PAPER, PAPER): 'Draw',
    (SCISSORS, SCISSORS): 'Draw',
    (PAPER, ROCK): 'Win',
    (SCISSORS, PAPER): 'Win',
    (ROCK, SCISSORS): 'Win',
    (ROCK, PAPER): 'Loss',
    (SCISSORS, ROCK): 'Loss',
    (PAPER, SCISSORS): 'Loss',
}

def main():
    print ('ROCK, PAPER, SCISSORS Game')
    tally = Counter()
    while True:
        print(f'{tally["Win"]} Wins  ||  {tally["Loss"]} Losses  ||  {tally["Draw"]} Draws')
        computer = get_computer_choice()
        player = get_player_choice()
        if player is None:
            sys.exit()
        else:
            outcome = OUTCOMES.get((player, computer))
            print(outcome)
            tally[outcome] += 1

if __name__ == '__main__':
    main()

Python has f-strings to simplify the creation of parameterized strings. As shown above in the printing of wins, losses, and draws.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ I woder how many rock paper scissor / tic tac toe / 4 in a row games we have written =p \$\endgroup\$ Jan 18 at 9:05
3
\$\begingroup\$

Solid work.

One thing you might consider doing is taking all of your program and wrapping it into a main() function. Then, at the bottom of the script, you can call the main() function in order to simplify the look of your code.

def main():
    pass

def turn_generation():
    pass

main()

Note how this simplifies the overall readability and makes it more concise. Better yet, your print call at the top can be placed in the main function in order to make your code more organized. Usually, you will opt to have your main function definition, other functions, then the function call in question.

As for function naming, try opting for something a little more representative of what is happening: turn_generation() could be called computer_choice().

A piece of syntax that might help you with your verbosity is placing the player and computer turns into a tuple to clean it up.

# start by taking the computer and player turns
turns = (player_turn, computer_turn)

if turns == ('r', 's'):
    pass

# continue on the logic from here

Note how this makes it more concise if you repeat less code.

\$\endgroup\$
1
  • 5
    \$\begingroup\$ It's not much to add if __name__ == '__main__': to your first paragraph, giving full best-practice. \$\endgroup\$ Jan 17 at 20:12

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.