2
\$\begingroup\$

After spending a few months learning python I decided to build a basic Rock Paper Scissors game.

Basic construct of the game:

  • Allows you to pick best of 3,5,7
  • Allows you to put in the name
  • Computer choices are completely random

I know there are probably a million different ways to write this code. But based on the way that I built it, are there any ways to make my code more efficient?

# Rock Paper Scissors

import random as rdm

print("Welcome to Rock/Paper/Scissors!!! \n")

gl = input("Would you like to play a best of 3, 5 or 7: ")
while gl not in ["3", "5", "7"]:
    gl = input("Incorrect Response, please select 3, 5, or 7: ")

gl = int(gl)

human_1 = input("Please enter your name: ")

GameOptions = ['Rock', 'Paper', 'Scissors']

hmn_score = 0
cpt_score = 0

rps_running = True


def rps():
    global cpt_score, hmn_score
    while rps_running:
        hmn_temp = input("""Please select from the following:
                                            1 - Rock
                                            2 - Paper
                                            3 - Scissors
                        \n""")
        while hmn_temp not in ["1", "2", "3"]:
            print("That was not a acceptable input!")
            hmn_temp = input("""Please select from the following:
                                    1 - Rock
                                    2 - Paper
                                    3 - Scissors
                \n""")

        hmn_final = int(hmn_temp) - 1

        print('You Chose: ' + GameOptions[hmn_final])

        cpt = rdm.randint(0, 2)

        print('Computer Chose: ' + GameOptions[cpt] + '\n')

        if hmn_final == cpt:
            print('Tie Game!')

        elif hmn_final == 0 and cpt == 3:
            print('You Win')
            hmn_score += 1
        elif hmn_final == 1 and cpt == 0:
            print('You Win')
            hmn_score += 1
        elif hmn_final == 2 and cpt == 1:
            print('You Win')
            hmn_score += 1
        else:
            print('You Lose')
            cpt_score += 1

        game_score()
        game_running()


def game_score():
    global cpt_score, hmn_score
    print(f'\n The current score is {hmn_score} for you and {cpt_score} for the computer \n')


def game_running():
    global rps_running, gl
    if gl == 3:
        if hmn_score == 2:
            rps_running = False
            print(f"{human_1} Wins!")

        elif cpt_score == 2:
            rps_running = False
            print(f"Computer Wins!")

        else:
            rps_running = True

    elif gl == 5:
        if hmn_score == 3:
            rps_running = False
            print(f"{human_1} Wins!")
        elif cpt_score == 3:
            rps_running = False
            print(f"Computer Wins!")
        else:
            rps_running = True

    elif gl == 7:
        if hmn_score == 4:
            rps_running = False
            print(f"{human_1} Wins!")
        elif cpt_score == 4:
            rps_running = False
            print(f"Computer Wins!")
        else:
            rps_running = True


rps()
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

Names

I've renamed:

  • hmn_* -> human_*
  • cpt_* -> computer_*
  • g1 -> max_score
  • human_1 -> human_name
  • game_score -> print_scores
  • game_running -> check_scores
  • rps -> start
  • rps_running -> running
  • rdm -> random
  • GameOptions -> GAME_OPTIONS (we usually use pascal case for a class in Python, and capitalization for variable constants)

Reason for all this renaming was to make it clear to an outside reader what all the variables mean without having to look past their declaration. If you ever revisit this code, you don't want to have to dive deep into it every time.

As for renaming the functions, we are now able to tell what they do without diving into their bodies. For example, print_scores clearly tells us that it prints the status of the scores.

check_scores

Currently you have three different outer ifs corresponding to max_score equaling 3, 5 or 7. In each of these ifs, you check if human_score or computer_score are greater than half of the total possible score. This entire function can be simplified by making this comparison work for any value of max_score:

def check_scores():
    global running, max_score

    if human_score > max_score / 2:
        running = False
        print(f"{human_1} Wins!")

    elif computer_score > max_score / 2:
        running = False
        print("Computer Wins!")

Since check_scores can only be possibly called if running == True, we don't need to reassign it to True in the else, so we can get rid of that.

start

You can make the input call for human_temp a function such that the prompt isn't specified twice in the code:

def get_human_temp():
    return input("""Please select from the following:
                        1 - Rock
                        2 - Paper
                        3 - Scissors
        \n""")

This changes the human_temp while loop to:

human_temp = get_human_temp()

while human_temp not in ["1", "2", "3"]:
    print("That was not a acceptable input!")
    human_temp = get_human_temp()

Ah, this is a do-while loop! If you're on Python 3.8, you can use the walrus operator:

while (human_temp := get_human_temp()) not in ["1", "2", "3"]:
    print("That was not a acceptable input!")

For your entire chain of elifs checking whether the human won, a more concise way to do is it to check if human_final - 1 == computer. This works for any human_final except 0. To make it work for 0, we would need to check human_final + 2 == computer. We can combine these two checks concisely as follows:

if human_final == computer:
    print('Tie Game!')

elif computer in (human_final - 1, human_final + 2):
    print('You Win')
    human_score += 1

else:
    print('You Lose')
    computer_score += 1

I believe that elif human_final == 0 and computer == 3 was a subtle bug in your original code, computer should have been checked against 2.

You can think of the elif as checking whether human_final is one ahead of computer in GAME_OPTIONS, while accounting for wrapping around GAME_OPTIONS.

Global State

There's a lot of global state in your program (show by all the global calls). We can use a class to store the state for each specific game. This state includes max_score (describes when a particular game ends), human_name (describes who is playing the game) and human_score / computer_score / running (describes current state of the game). Let's call this class Game, with an initialization method like this:

def __init__(self, max_score, human_name):
    self.max_score = max_score
    self.human_name = human_name

    self.human_score = 0
    self.computer_score = 0
    self.running = False

We would then put all of your methods using global state in Game, with self prepended to all the variables we have here in our __init__ method.

As for the code that is run before we even start the game (the code responsible for fetching human_name and max_score), we can put this in an if __name__ == "__main__" block. This makes it so we can use Game from another module without having all the input-specific code running.

Here's the final code, with some very slight consistency changes (such as standardizing the type of quotation mark you use, and getting rid of unnecessary fs at the beginning of unformatted strings):

# Rock Paper Scissors

import random


def get_human_temp():
    return input("""Please select from the following:
                        1 - Rock
                        2 - Paper
                        3 - Scissors
        \n""")


GAME_OPTIONS = ["Rock", "Paper", "Scissors"]


class Game:
    def __init__(self, max_score, human_name):
        self.max_score = max_score
        self.human_name = human_name

        self.human_score = 0
        self.computer_score = 0
        self.running = False

    def print_scores(self):
        print(f"\n The current score is {self.human_score} for you and {self.computer_score} for the computer \n")

    def check_scores(self):
        if self.human_score > self.max_score / 2:
            self.running = False
            print(f"{self.human_name} Wins!")

        elif self.computer_score > self.max_score / 2:
            self.running = False
            print("Computer Wins!")

    def start(self):
        self.running = True

        while self.running:
            while (human_temp := get_human_temp()) not in ["1", "2", "3"]:
                print("That was not a acceptable input!")

            human_final = int(human_temp) - 1
            print(f"You Chose: {GAME_OPTIONS[human_final]}")

            computer = random.randint(0, 2)
            print(f"Computer Chose: {GAME_OPTIONS[computer]}\n")

            if human_final == computer:
                print("Tie Game!")

            elif computer in (human_final - 1, human_final + 2):
                print("You Win")
                self.human_score += 1

            else:
                print("You Lose")
                self.computer_score += 1

            self.print_scores()
            self.check_scores()


if __name__ == "__main__":
    print("Welcome to Rock/Paper/Scissors!!! \n")

    max_score = input("Would you like to play a best of 3, 5 or 7: ")

    while max_score not in ["3", "5", "7"]:
        max_score = input("Incorrect Response, please select 3, 5, or 7: ")

    max_score = int(max_score)
    human_name = input("Please enter your name: ")

    game = Game(max_score, human_name)
    game.start()
\$\endgroup\$
1
  • 1
    \$\begingroup\$ Wow this is fantastic! Thank you so much for spending the time to not only go through the code but to rewrite it. I will definitely take what you've outlined and use it to improve on subsequent projects. \$\endgroup\$
    – Sidwho
    Jun 8, 2020 at 19:29

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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