current community

your communities

more stack exchange communities

Stack Exchange Inbox Reputation and Badges
tour help
Sign up ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 6 down vote favorite

I'm new to programming and I'm trying to teach myself Python through some online books.

One of the exercises has me making a Rock Paper Scissors game. The one I have written works well enough I think, but I was wondering if there were more efficient ways of writing the code, perhaps something shorter.

I'm using Python 3.4.3 IDLE GUI.

    import random

#variables
done = False
comp = random.randrange(1,4)

#start
print("Lets play rock, paper, scissors! Choose numbers 1-3 only please.")
print()

#begin loop

while not(done):
#computer vs input
    guess = int(input("1. Rock 2. Paper 3. Scissors "))
    if comp == 1 and guess == 3:
        print("Rock beats scissors, too bad!")
    elif comp == 2 and guess == 1:
        print("Paper beats rock, tough luck!")
    elif comp == 3 and guess == 2:
        print("Scissors me timbers! Scissors cuts paper! You lose.")
    elif guess == 1 and comp == 3:
        print("Rock beats scissors, you beat me!")
    elif guess == 2 and comp == 1:
        print("You win! Paper beats rock.")
    elif guess == 3 and comp == 2:
        print("Scissor cuts paper, you win!")
    else:
        print("It's a draw!")

#continue       
    stop = input("Do you want to quit? ")
    if stop.lower() == "y" or stop.lower() == "yes":
        done = True
share|improve this question
    
Welcome to Code Review! I hope you get some great answers! –  Phrancis yesterday

1 Answer 1

up vote 5 down vote accepted

Looping

Rather than setting a done flag and looping while not(done): (note: this would usually be written as simply while not done:), you could use a formulation more like:

while True:
    # do things
    if loop_should_end:
        break

This removes the need for the flag and makes the loop easier to follow.

Logic

At present, comp is set outside the loop, so the computer will always guess the same thing. This makes it rather too easy to win - you should move comp = random.randint(1, 4) (note whitespace) inside the loop.

Comparisons

The most obvious tweak is that the canonical replacement for lots of elifs in Python is a dictionary. In this case, you could look up a tuple (comp, guess) and print the result:

RESULTS = {
    (1, 3): "Rock beats scissors, too bad!",
    (2, 1): "Paper beats rock, tough luck!",
    ...
}

then access this inside the loop as easily as:

print(RESULTS.get((comp, guess), "It's a draw!"))

You could also simplify:

stop = input("Do you want to quit? ")
if stop.lower() == "y" or stop.lower() == "yes":

to:

if input("Do you want to quit? ").lower() in {"y", "yes"}:

Validation

You currently have no input validation, so the program could crash with a ValueError if the user types 'scissors' instead of 3. See e.g. "Asking the user for input until they give a valid response" for guidance on this.

Structure

Finally, it is bad practice to have all of your code running at the top level of the script - it makes it much more difficult to re-use later. Instead, I would do something like:

import random

RESULTS = {...}  # possible outcomes

def get_int_input(...):
    """Take valid integer input from user."""
    ...

def game_loop(...):
    """Main game loop."""
    ...

if __name__ == '__main__':
    game_loop()

This will work exactly the same if you run the script directly (see "What does if __name__ == “__main__”: do?") but allows you to use e.g. from wherever import get_input_int elsewhere, simplifying reuse and encouraging modular design.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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