Rock, Paper, Scissors - Python game

Question

This is my attempt at making a 'rock, paper, scissors' game in python. The game takes the user's input and the computer's random selection and calculates who has won. All feedback appreciated.

from random import randint

def getUser():
    print('Rock, Paper, Scissors: ') 
    userIn = input().lower()
    if userIn == 'r':
        userIn = 0        
    elif userIn == 'p':
        userIn = 1        
    elif userIn == 's':
        userIn = 2
    else:
        getUser()
    return(userIn)

def getComp():
    compIn = randint(0,2)
    return(compIn)

def calc(user, comp):  
    if user == comp:
        print("Draw!")
    elif user == 0:
        if comp == 1:
            print("You lose - ",  comp, "smothers", user)
        else:
            print("You win -", user, "smashes", comp)
    elif user == 1:
        if comp == 2:
            print("You lose - ", comp, "slices", user)
        else:
            print("You win -", user, "smothers", comp)
    elif user == 2:
        if comp == 0:
            print("You lose - ", comp, "smashes", user)
        else:
            print("You win -", user, "slices", comp)

again = 'yes'
while again != 'no':
    calc(getUser(), getComp())
    again = input('Would you like to play again? ')
print('Thank you for playing.')

Answer 1

Like @wb9688 mentioned in the comments, the way you currently determine the winner is fine and working, but it does not scale very well. If you wanted to add two more possible moves (for example lizard and spock), your if..else tree will become very large and hard to maintain.

It would therefore be better to just save which move beats which other move (for example in a dictionary):

beats = {'paper': 'scissors',
         'rock': 'paper',
         'scissors': 'rock'}

Then you can just do:

computer_choice = getComp()
user_choice = getUser()

if user_choice == beats[computer_choice]:
    print "{} beats {}, you win!".format(user_choice, computer_choice)
elif computer_choice == beats[user_choice]:
    print "{} beats {}, computer wins!".format(computer_choice, user_choice)
else:
    print "Tie."

If you also want to add the verbs, just add a dictionary for that as well:

verb = {'rock': 'smashes',
        'paper': 'smothers',
        'scissors': 'slices'}

And then do:

if user_choice == beats[computer_choice]:
        print "{} {} {}, you win!".format(user_choice, verb[user_choice], computer_choice)
...

You can have a look at my answer to a similar question on how easy it is to extend this framework to more moves: Stone, Paper, Scissors in Python

Note that you would have to change the structure of verb to a nested dictionary, because "Paper disproves Spock" instead of "Paper smothers Spock".

Your function getUser should be modified, as well. Right now, you call it recursively in case the user enters an illegal move. However, you never return the result of that recursive call!

In python, using a while loop is usually preferred in this case, because Python does not do tail recursion and so will hit a maximum recursion depth at some point:

def getUser():
    while True:
        user_input = raw_input("Make a choice: {} ".format(beats.keys()))
        if user_input in beats:
            return user_input
        print "Invalid choice."

Note: In Python-3.x, use input instead of raw_input and print(x) instead of print x.

Finally, in your function getComp, you can just return the result right away. However, it needs to be modified to use the keys of beats, instead of integers:

import random

def getComp():
    return random.choice(beats.keys())

Answer 2

You want less static code. Take calc, you want to change it so you only have to define one print. And so the aim is:

print('You {} - {} {} {}'.format('win' if user_wins else 'lose', winner, ACTION[winner], looser))

Working backwards you should know who the winner and looser are if you know if the user wins. Using tuple unpacking and the ternary operator you do this in one line:

winner, looser = (user, comp) if user_wins else (comp, user)

We then want you code to find user_wins, if we stay with your current approach then we should get six ifs:

user_wins = True
if user == 0:
    if comp == 1:
        user_wins = False
elif user == 1:
    if comp == 2:
        user_wins = False
elif user == 2:
    if comp == 0:
        user_wins = False

We can actually simplify this. But it's easier to say it defaults to False, and we need to change it to True. This makes the first computer if, if comp == 2 rather than if comp == 1. We then want to transform these to use and and or. Which should get you:

user_wins = (
    (user == 0 and comp == 2)
 or (user == 1 and comp == 0)
 or (user == 2 and comp == 1)
)

This should remind you of lists, and in. And so you can make that code a list of tuple of ints, and use in. Which can result in:

user_wins = (user, comp) in [(0, 2), (1, 0), (2, 1)]

Which drastically simplifies your function.

---

You should also use more constants. The list from above, [(0, 2), (1, 0), (2, 1)], should be one. Same with the numbers 0, 1, and 2. This is as it's easier to understand user == ROCK rather than user == 0. And so I'd move the actions, smothers, smashes, and slices, to become a list. The winning moves from above. The moves should also be constants. And so you should have:

ROCK = 0
PAPER = 1
SCISSORS = 2
ACTION = ["smashes", "smothers", "slices"]
WIN_CONDITIONS = [(ROCK, SCISSORS), (PAPER, ROCK), (SCISSORS, PAPER)]

---

Finally I'd say you should simplify your input to move code, getUser. First, to follow the same style as a lot of Python programmers you should rename it to get_user. After this, you could use a datatype that's whole point is to simplify lookup. In Python this is called a dictionary, aka dict.

This stores a key and a value, we could input the key 'r' to output '0'. Which can look like {'r': ROCK}. After this we can add the two other moves.

However we'll have to use dict.get, so that if it's not in the dictionary, we'll get None. This will allow us to implement the else, of your current function. But, I would change it so that it doesn't use recursion, as recursion has a limit. Instead of doing getUser() I'd use a while True loop. And have an if where if the input, after going through the dict, is not None, we return the input.

---

All this together can get you:

from random import randint

ROCK = 0
PAPER = 1
SCISSORS = 2
ACTION = ["smashes", "smothers", "slices"]
WIN_CONDITIONS = [(ROCK, SCISSORS), (PAPER, ROCK), (SCISSORS, PAPER)]
USER_INPUT = {
    'r': ROCK,
    'rock': ROCK,
    'p': PAPER,
    'paper': PAPER,
    's': SCISSORS,
    'scissors': SCISSORS,
}


def getUser():
    while True:
        print('Rock, Paper, Scissors: ')
        user_input = USER_INPUT.get(input().lower())
        if user_input is not None:
            return user_input


def display_winner(user, comp):
    if user == comp:
        print("Draw!")
    else:
        user_wins = (user, comp) in WIN_CONDITIONS
        winner, looser = (user, comp) if user_wins else (comp, user)
        print('You {} - {} {} {}'.format('win' if user_wins else 'lose', winner, ACTION[winner], looser))


if __name__ == '__main__':
    again = 'yes'
    while again != 'no':
        display_winner(getUser(), randint(0, 2))
        again = input('Would you like to play again? ')
    print('Thank you for playing.')