5
\$\begingroup\$

This is the biggest program I have ever made. I created a Rock Paper Scissors game. It asks you if you want to play, takes your input, compares it with the random computer input, gives you the outcome, and then asks if you want to play again.

I am POSITIVE that I could have done this in fewer lines of code, so I would absolutely appreciate feedback. I'm brand new to Python. I am currently using Python 3.

import random
import sys
#begin the game and then loop after the first play.

def play():
    while True:
        p_choice = input("What do you choose?")
        cpu_random = random.randint(1,3)
        cpu_choice = cpu_random
        if cpu_random == 1:
            cpu_choice = "Rock"
        elif cpu_random == 2:
            cpu_choice = "Paper"
        elif cpu_random == 3:
            cpu_choice = "Scissors"

#Compare the data given by the user to the CPU

        def compare():

            play_again = None
#Tie outcome
            if p_choice == cpu_choice:
                print("Tie!")
                play_again = input("Play again?")

#Rock outcome

            elif p_choice == "Rock" and cpu_choice == "Paper":
                print("You Lose!")
                play_again = input("Play again?")
            elif p_choice == "Rock" and cpu_choice == "Scissors":
                print("You Win!")
                play_again = input("Play again?")

#Paper outcome

            elif p_choice == "Paper" and cpu_choice == "Scissors":
                print("You Lose!")
                play_again = input("Play again?")
            elif p_choice == "Paper" and cpu_choice == "Rock":
                print("You Win!")
                play_again = input("Play again?")

#Scissors outcome

            elif p_choice == "Scissors" and cpu_choice == "Rock":
                print("You Lose!")
                play_again = input("Play again?")
            elif p_choice == "Scissors" and cpu_choice == "Paper":
                print("You Win!")
                play_again = input("Play again?")

#Ask if you want to play again, then give input

            if play_again == "Yes":
                play()
            elif play_again == "No":
                print("Game Over")
                sys.exit()
            else:
                print("Please try again")
                play_again = input("play again?")
                return play_again

        compare()



#ask if player wants to start
def game_start():
    while True:
        begin = input("Would you like to play Rock, Paper, Scissors?")
        if begin == "Yes":
            play()
            return begin
        while begin != "Yes":
            if begin == "No":
                print("Game Over")
                return begin
            else:
                print("Please try again")
                break

game_start()
\$\endgroup\$
4
\$\begingroup\$

I would use a dictionary to store the choices, and pass the input through capitalize() so, the user doesn't have to write the capitalized word(i am doing this only because you have it that way, but unless you are printing the word as it is, i recommend you to use only lowercase or uppercase to avoid unmatching), also, the while loop is unnecesary because you are calling play() if the user wants to play again.

def play():
    p_choice = input("What do you choose?").capitalize()
    choices = {1 : 'Rock', 2 : 'Paper', 3 : 'Scissors'}
    cpu_choice = choices[random.randint(1,3)]

then, you can do the same thing for all cases, instead of all those conditionals, you can build a dictionary for all cases and then let the compare function return True if its a win or False if its a lose, although i wouldn't recommend you to define the function inside play()

def compare(playerChoice,cpuChoice):
    results = {('Paper','Rock') : True,
               ('Paper','Scissors') : False,
               ('Rock','Paper') : False,
               ('Rock','Scissors') : True,
               ('Scissors','Paper') : True,
               ('Scissors','Rock') : False}
    return results[(playerChoice,cpuChoice)]

and this is my version of game_start()

def game_start():
    begin = input("Would you like to play Rock, Paper, Scissors? ").capitalize()
    while begin != "Yes":
        if begin == "No":
            print("Game Over")
            return sys.exit()
        else:
            print("Please try again")
            begin = input("Would you like to play Rock, Paper, Scissors? ").capitalize()
    play()
    while True:
        begin = input('Play again?').capitalize()
        while begin != "Yes":
            if begin == "No":
                print("Game Over")
                sys.exit()
            else:
                print("Please try again")
                begin = input("Play again? ").capitalize()
        play()

the whole code looks now like this:

import random
import sys

def play():
    p_choice = input("What do you choose?").capitalize()
    choices = {1 : 'Rock', 2 : 'Paper', 3 : 'Scissors'}
    cpu_choice = choices[random.randint(1,3)]
    if p_choice == cpu_choice:
        return print('Tie!')
    if compare(p_choice,cpu_choice):
        return print('You Win!')
    else:
        return print('You Lose!')

def compare(playerChoice,cpuChoice):
    results = {('Paper','Rock') : True,
               ('Paper','Scissors') : False,
               ('Rock','Paper') : False,
               ('Rock','Scissors') : True,
               ('Scissors','Paper') : True,
               ('Scissors','Rock') : False}
    return results[(playerChoice,cpuChoice)]

def game_start():
    begin = input("Would you like to play Rock, Paper, Scissors? ").capitalize()
    while begin != "Yes":
        if begin == "No":
            print("Game Over")
            sys.exit()
        else:
            print("Please try again")
            begin = input("Would you like to play Rock, Paper, Scissors? ").capitalize()
    play()
    while True:
        begin = input('Play again?').capitalize()
        while begin != "Yes":
            if begin == "No":
                print("Game Over")
                sys.exit()
            else:
                print("Please try again")
                begin = input("Play again? ").capitalize()
        play()

game_start()            
\$\endgroup\$
  • \$\begingroup\$ You could avoid repeating yourself in the game_start function by writing a ask_yes_or_end_game(message) function. \$\endgroup\$ – Graipher Aug 8 '17 at 12:43
2
\$\begingroup\$

You want a helper function, a comparator:

RPS = ['rock', 'paper', 'scissors']

def cmp(g1, g2):
    '''For two guesses, return 1 if g1 is winner, -1 for lose, 0 for tie.'''
    i1 = RPS.index(g1)
    i2 = RPS.index(g2)
    d = (i1 - i2) % len(RPS)
    return -1 if d == 2 else d

You can test it in this way:

def reflexive(winner, verb, loser):
    assert 1 == cmp(winner, loser)
    assert -1 == cmp(loser, winner)


if __name__ == '__main__':
    for guess in RPS:
        assert  0 == cmp(guess, guess)
    reflexive('rock', 'beats', 'scissors')
    reflexive('scissors', 'cuts', 'paper')
    reflexive('paper', 'covers', 'rock')
\$\endgroup\$
1
\$\begingroup\$

Annotated code below. It can be shortened, I'll add a shot version later

import random

def play():
    continue_playing = True
    while continue_playing:  # You already have a loop so no need to call play() in the function
        p_choice = input("What do you choose?")
        # You need to validate user input too
        while p_choice not in ("Rock", "Paper", "Scissors"):
            print("Please try again")
            p_choice = input("What do you choose?")
        cpu_random = random.choice(("Rock", "Paper", "Scissors"))
        # Much clearer than randint
        # Here I use tuples, which are basically lists you can't modify
        # Python will optimize these in certain cases so use them instead of lists for things you never change

        # Defining compare() is useless since you only call it once anyway
        # Comments are only for explanation, if your code is simple to understand try to avoid comments. For example, people can see if you compare to "Rock" that it's the logic for the rock outcomes
        if p_choice == cpu_choice:
            print("Tie!")
        elif p_choice == "Rock" and cpu_choice == "Paper":
            print("You Lose!")
        elif p_choice == "Rock" and cpu_choice == "Scissors":
            print("You Win!")
        elif p_choice == "Paper" and cpu_choice == "Scissors":
            print("You Lose!")
        elif p_choice == "Paper" and cpu_choice == "Rock":
            print("You Win!")
        elif p_choice == "Scissors" and cpu_choice == "Rock":
            print("You Lose!")
        elif p_choice == "Scissors" and cpu_choice == "Paper":
            print("You Win!")
        play_again = input("Play again?")
        while play_again not in ("Yes", "No"):  # validate user input
            print("Please try again")
            play_again = input("Play again?")
        if play_again == "No":
            print("Game Over")
            continue_playing = False
        # Don't return here (you returned the input), I really don't think that's what you wanted

def game_start():
    while True:
        begin = input("Would you like to play Rock, Paper, Scissors?")
        if begin == "Yes":
            play()
        elif begin == "No":
            print("Game Over")
        else:
            print("Please try again")
        # No need for return or break here, if-else is enough

if __name__ == "__main__":  # Only run this if run from commandline
    game_start()
\$\endgroup\$
  • \$\begingroup\$ But it's the same code, just edited (and commented) where I changed things, is there any way this is not clear/any way I could improve this answer? \$\endgroup\$ – somebody Aug 8 '17 at 6:43
  • 1
    \$\begingroup\$ My bad, yeah it's actually fine. But comments embedded in code are easy to overlook. I'd recommend to use the more common format of speaking in paragraphs, with code snippets highlighting the bad parts and their remedies. \$\endgroup\$ – janos Aug 8 '17 at 7:35

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.