HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Scissors, Paper, Rock in Python

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
paperscissorsrockpython

Problem

The game works, but I was wondering if anyone could help me with better structure for the code, simplification, or just any other improvements.

from random import randrange

# assigns the amount of turns and starts the program
def Main():  
    print "~A game of Scissors, Paper, Rock!~"
    global turns
    turns = raw_input("How many times do you want to play? : ")
    # tests for int input and restarts if not an int
    try:
        turns = int(turns)
        print "\nYou will play", turns, "turn(s) against the computer! \n"
        play()
    except ValueError:
        print "That's not a valid input! Integers only, please! \n"
        Main()

#determines the player's choice and sets the computers choice
def play():
    global options
    options = {1: "Scissors", 2: "Paper", 3: "Rock"}
    global turns
# loops for different choices, and calls the comparison function
    while turns > 0:
        try:
            global pchoice
            pchoice = int(raw_input("Choose one! \n  Scissors \n  Paper \n  Rock \n"))
            global cchoice
            cchoice = randrange(1,4)
            comparison()
        except KeyError:
            print "an invalid input! Please try again. \n"
            play()
        turns -= 1

# compares player's choice with the computer's and returns a tie, win, or loss
def comparison():
    print "You chose:", options[pchoice], "and the computer chose:", options[cchoice]
    if int(pchoice) == cchoice:
        print "A tie! \n"
    elif pchoice == 1 and cchoice == 3 or pchoice == 2 and cchoice == 1 or pchoice == 3 and cchoice == 2:
        print "You lose! \n"
    else:
        print "You win! \n"

Main()

Solution

I'd like to focus on two key issues:

Global variables

You should strive to have no global variables in your program, and it is entirely possible to eliminate them from this program.

If your functions only ever use local variables, then you can understand your code by analyzing each function separately. However, when you use global variables, you will need to analyze the entire program at once, because the values could be affected by code anywhere in the program.

For example, there is no reason why turns needs to be global. By making it global, you cannot be sure, just by looking at the play() function, that the loop will execute turns number of times, because you have to worry about the possibility that comparison() modifies its value. Rather, turns should be local to play(). (Actually, it should be a parameter.)

def play(turns):
    # loops for different choices, and calls the comparison function
    while turns > 0:
        try:
            pchoice = int(raw_input("Choose one! \n  Scissors \n  Paper \n  Rock \n"))
            cchoice = randrange(1,4)
            comparison(pchoice, cchoice)
        except KeyError:
            print "an invalid input! Please try again. \n"
            play(turns)     # Problematic.  I'll say more about this later…
            return
        turns -= 1


Use of functions as if they were GOTO labels

Suppose that at the "How many times do you want to play?" prompt, you hit Enter many times, then hit ControlC. You would get a stack trace like this:

How many times do you want to play? : ^CTraceback (most recent call last):
File "cr64216.py", line 45, in
Main()
File "cr64216.py", line 15, in Main
Main()
File "cr64216.py", line 15, in Main
Main()
File "cr64216.py", line 15, in Main
Main()
File "cr64216.py", line 15, in Main
Main()
File "cr64216.py", line 15, in Main
Main()
File "cr64216.py", line 15, in Main
Main()
File "cr64216.py", line 15, in Main
Main()
File "cr64216.py", line 15, in Main
Main()
File "cr64216.py", line 15, in Main
Main()
File "cr64216.py", line 15, in Main
Main()
File "cr64216.py", line 15, in Main
Main()
File "cr64216.py", line 7, in Main
turns = raw_input("How many times do you want to play? : ")
KeyboardInterrupt




That deep stack trace is a sign of improper recursion. A properly constructed program should only have one frame in the stack at that point, namely Main().

Rather than recursing, you should implement the retry mechanism using a loop.

def Main():
print "~A game of Scissors, Paper, Rock!~"

# Ask until a valid number of turns is given by the user
while True:
turns = raw_input("How many times do you want to play? : ")
# tests for int input and restarts if not an int
try:
turns = int(turns)
print "\nYou will play", turns, "turn(s) against the computer! \n"
break
except ValueError:
print "That's not a valid input! Integers only, please! \n"

play(turns)

Code Snippets

def play(turns):
    # loops for different choices, and calls the comparison function
    while turns > 0:
        try:
            pchoice = int(raw_input("Choose one! \n <1> Scissors \n <2> Paper \n <3> Rock \n"))
            cchoice = randrange(1,4)
            comparison(pchoice, cchoice)
        except KeyError:
            print "an invalid input! Please try again. \n"
            play(turns)     # Problematic.  I'll say more about this later…
            return
        turns -= 1

Context

StackExchange Code Review Q#64216, answer score: 9

Revisions (0)

No revisions yet.