0
\$\begingroup\$

as a beginner in Python I made a rock, paper, scissors game and was wondering if my code was a bit weird and could be simplified???

print("Rock, Paper, Scissors")

def rps():
userInput = input("Please enter your choice: ")
userInput = userInput.lower()
if userInput != 'rock' and userInput != 'paper' and userInput != 'scissors':
    print("Your choice is invalid")
import random
computerChoice = ''
for x in range(1):
    num = random.randint(1,4)
if num == 1:
    computerChoice = 'rock'
elif num == 2:
    computerChoice = 'paper'
elif num == 4:
    computerChoice = 'scissors'
if userInput == computerChoice:
    print("DRAW!")
elif userInput == 'rock' and computerChoice == 'paper':
    print("COMPUTER WINS!")
elif userInput == 'rock' and computerChoice == 'scissors':
    print('USER WINS!')
elif userInput == 'paper' and computerChoice == 'rock':
    print("USER WINS!")
elif userInput == 'paper' and computerChoice == 'scissors':
    print("COMPUTER WINS!")
elif userInput == 'scissors' and computerChoice == 'rock':
    print("COMPUTER WINS!")
elif userInput == 'scissors' and computerChoice == 'paper':
    print("USER WINS!")
print("Would you like to try again?")
try_again = input("Y/N  ")
try_again = try_again.lower()
if try_again == "y":
    print("\n")
    rps()
else:
    print("\n")
    print("Thank you for using my Rock, Paper, Scissors Program")

rps()

\$\endgroup\$
  • 2
    \$\begingroup\$ Welcome to Code Review! There seems to be a major issue with the indentation of your code. Since indentation is of key importance in Python, please make sure to reformat your code. Copy it as is from your editor, select all of it and press Ctrl+k to make it a properly formatted code block. \$\endgroup\$ – AlexV Jul 17 at 7:13
  • \$\begingroup\$ also check the responses on the dozens of other "rock, paper, scissors" questions on Code Review \$\endgroup\$ – Maarten Fabré Jul 17 at 10:08
3
\$\begingroup\$

First I suggest you to split your logic into several function, it will help you to see what is essential and what you could optimize / remove.

import random
print("Rock, Paper, Scissors")

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


def rps():
  try:
    userInput = getUserInput()
    computerChoice = getComputerChoise()
    result = getResult()
    tryAgainMaybe()
  except Exception as err:
    print(err)


  def getUserInput() :
    userInput = input("Please enter your choice: ")
    userInput = userInput.lower()
    if userInput not in AVAILABLE_CHOICES:
        raise Exception('Your choice is invalid')
    return userInput;

  def getComputerChoice():
    return random.choice(AVAILABLE_CHOICES);

  def getResult():
    result = ''

    if userInput == computerChoice:
      result = "DRAW!"
    elif userInput == 'rock'     and computerChoice == 'scissors' or \
         userInput == 'paper'    and computerChoice == 'rock' or \
         userInput == 'scissors' and computerChoice == 'paper':
        result = 'USER WINS!'
    else:
        result = "COMPUTER WINS!"
    return result

  def tryAgainMaybe():
    print("Would you like to try again?")
    try_again = input("Y/N  ")
    try_again = try_again.lower()
    if try_again == "y":
        print("\n")
        rps()
    else:
        print("\n")
        print("Thank you for using my Rock, Paper, Scissors Program")
  1. Create a list of available choices instead of writing them everywhere. it will be easier to modify and to test for your input values.

  2. random allow you to select ramdomly a value in a list.

  3. you don't need to calculate every single possibility. You can either have a draw, a win or a loose. if you don't win or draw, you loose, no computation needed here.

  4. when you name something, try to use the same name for the same thing everything, otherwise it could be missleading. eg (userInput / computerChoice) why do we have input and choice two different words for the same kind of value ? i would choose userChoice and computerChoice instead.

  5. Your coding convention does not respect pep8 standard. You should be using snake_case.

\$\endgroup\$
  • 1
    \$\begingroup\$ Python convention (PEP-8) is snake_case for functions. Apart from that rps is a very unclear function name. Pyhton does not slow down if the variable names get longer \$\endgroup\$ – Maarten Fabré Jul 17 at 10:08
  • \$\begingroup\$ Indeed, but i followed the case convention provided in the question to not add confusion. I didn't rename the existing variable neither as it could confuse op. i'll write something about it \$\endgroup\$ – PyNico Jul 17 at 10:10
2
\$\begingroup\$

I don't mind the other two answers but I felt like there was a better way to do this. Consider using a dictionary where the key will be the selection and the value will be what that selection beats.

So the dict will be:

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

This dict can simplify a lot of our logic.

First lets get the user's choice. The following loop makes sure the user_choice is a valid choice by making sure it is in our dict.

user_choice = None
while user_choice not in choices:
    user_choice = input(f'Please enter one of the following ({", ".join(choices)}): ')

Note: The ', '.join(iterable) notation may be confusing. All we are doing is joining all the dictionary keys together to make one string using ', ' as the delimiter. This results in the following output:

Please enter one of the following (paper, rock, scissors):

Now lets let the computer choose an option. Similar to Josay's answer, we can use random's choice to do this; however, because a dictionaries keys are basically a set we need to first turn our keys into a list (or tuple).

from random import choice
computer_choice = choice(list(choices))

Great, now we have both choices. Now for the winning logic, which gets simplified because of our choices dict:

if user_choice == computer_choice: # If both choices are the same then its a draw
    print("DRAW")
elif choices[user_choice] == computer_choice: # If the computer's choice is what the user's choice beats
    print("PLAYER WINS")
else: # else the computer must have won
    print("COMPUTER WINS")

Putting it all together:

from random import choice

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

user_choice = None
while user_choice not in choices:
    user_choice = input(f'Please enter one of the following ({", ".join(choices)}): ')

computer_choice = choice(list(choices))
print(f"The computer chose {computer_choice}")

if user_choice == computer_choice:
    print("DRAW")
elif choices[user_choice] == computer_choice:
    print("PLAYER WINS")
else:
    print("COMPUTER WINS")

Final output:

Please enter one of the following (paper, rock, scissors): paper
The computer chose rock
PLAYER WINS
\$\endgroup\$
1
\$\begingroup\$

Getting the computer choice

Many things can be improved in the way the computer choice is computed:

import random
computerChoice = ''
for x in range(1):
    num = random.randint(1,4)
if num == 1:
    computerChoice = 'rock'
elif num == 2:
    computerChoice = 'paper'
elif num == 4:
    computerChoice = 'scissors'

Here are different steps to make things better:

  • Useless range: I don't really understand the point of for x in range(1).

  • Uncaught bug: the case num == 3 is not handled in any way. My guess is that both num = random.randint(1,4) and elif num == 4: should use a 3 rather than a 4. Also, when that probleme occurs, we end up with computerChoice = '' which stays undetected until the end of the program.

  • Data structure over code: as mentionned above, there is an issue in the code mapping an integer value to a string value. You could have used a data structure to perform that operation rather than a bunch of if...elif... and that issue would have been detected earlier.

This could be done with a list (and some re-indexing):

cpuChoices = ['rock', 'paper', 'scissors']
computerChoice = cpuChoices[random.randint(0, 2)]

or a dictionnary:

cpuChoices = {1: 'rock', 2: 'paper', 3: 'scissors'}
computerChoice = cpuChoices[random.randint(1, 3)]
  • The right tool for the right job: the Python standard library contains many nice tools and sometimes there is a perfect fit for what you are trying to achieve. This is particularly true for tasks which are common enough. In your case, there is: random.choice.
cpuChoices = ['rock', 'paper', 'scissors']
computerChoice = random.choice(cpuChoices)

To be continued

\$\endgroup\$
  • \$\begingroup\$ Thanks to everyone who answered my question! It really helped expand my knowledge of Python and as this was one of my first projects and am thankful that so many answers were given. I see now that python has a lot more things for (in the future) my projects to be much, much, much more efficient! Once again thanks to everyone who answered my question! \$\endgroup\$ – oscarfrederiksen Jul 18 at 14:01
0
\$\begingroup\$

Suggestions

In addition to the suggestions provided by others, I observations on how to improve your code:

  1. Problems like these, where we both have some information that needs to be stored and have a bundle of functions closely related can be effectively made into a class.
  2. The problem is cyclic over the 3 options rock, paper, and scissors, which means we can represent them as numbers and comparisons on between them are constant over modulus 3.

Suggested Implementation

If we put these principles into practise we get the following form:

import random

class RPS:

    options = {
        'rock':0
        'paper':1
        'scissors':2
    }
    responses = ('DRAW!','PLAYER WINS!','COMPUTER WINS!')
    prompt_string = 'Please enter your choise of {}: '.format(', '.join(options.keys()))

    def __init__(self,prompt=input, out=print):
        self.prompt = prompt
        self.out = out

    def __call__(self):
        self.run()

    def run(self):
        self.single_round()
        while self.want_more():
            self.out('\n')
            self.single_round()
        self.out('\n')
        self.out("Thank you for using my Rock, Paper, Scissors Program")

    def want_more(self):
        self.out("Would you like to try again?")
        return self.prompt("Y/N  ").lower() == "y"

    def single_round(self):
        player = self.prompt_choice()
        computer = random.randint(0,2)
        self.out(self.responses[(player - computer) % 3])

    def prompt_choise(self):
        choise = self.prompt(prompt_string).lower()
        while choise not in options:
            self.out('Your choice is invalid')
            choise = self.prompt(prompt_string).lower()
        return self.options[choise]

if __name__ == "__main__":
    RPS().run()

Note how easy we can get the proper response by simply refering to it as index, now that we can transform all the results into the same relative region (did the player have the same, go one further in the cycle or one less in the cycle, since the one just further in the cycle beats the previous one).

\$\endgroup\$

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.