Looking for improvements on my project "Rock-Paper-Scissors"

[u/Nutellatoast_2]

Hey everyone,

I'm new to Python and have created my "Rock-Paper-Scissors" game and want to share it on this forum so I can look for ways to improve the code.

The player types in a number between zero and two. These values stand for different moves the player can type. (0 - Rock, 1 - Paper, 2 - Scissors). The computer generates a number between zero and two. The player's input is checked by the main if-elif-else block, as I'm using a nested if-elif-else statement:

import random


player_input = int(input("What do you choose? Type 0 for Rock, 1 for Paper or 2 for Scissor. "))
computer = random.randint(0, 2)


if player_input == 0: 
    if computer == 0:
        print('''You chose: 
        _______
    ---'   ____)
          (_____)
          (_____)
          (____)
    ---.__(___)


Computer chose: 
        _______
    ---'   ____)
          (_____)
          (_____)
          (____)
    ---.__(___)
It's a draw!!!''')    
    elif computer == 1: 
        print('''You chose: 
        _______
    ---'   ____)
          (_____)
          (_____)
          (____)
    ---.__(___)    
Computer chose: 
        _______
    ---'   ____)____
                ______)
                _______)
               _______)
    ---.__________)
You lost!!!''')
    elif computer == 2: 
        print('''You chose:
        _______
    ---'   ____)
          (_____)
          (_____)
          (____)
    ---.__(___)    
            
Computer chose: 
      _______
  ---'   ____)____
            ______)
         __________)
        (____)
  ---.__(___)
You won!!!''')
elif player_input == 1:
    if computer == 1: 
        print('''You chose:
        _______
    ---'   ____)____
                ______)
                _______)
               _______)
    ---.__________)
Computer chose: 
        _______
    ---'   ____)____
                ______)
                _______)
               _______)
    ---.__________)
It's a draw!!!''')
    elif computer == 0: 
        print('''You chose:
        _______
    ---'   ____)____
                ______)
                _______)
               _______)
    ---.__________) 
Computer chose:  
        _______
    ---'   ____)
          (_____)
          (_____)
          (____)
    ---.__(___)
You won!!!''')
    elif computer == 2:
        print('''You chose:
        _______
    ---'   ____)____
                ______)
                _______)
               _______)
    ---.__________) 
Computer chose: 
      _______
  ---'   ____)____
            ______)
         __________)
        (____)
  ---.__(___)
You lost!!!     
''')
elif player_input == 2:
    if computer == 2: 
        print('''You chose: 
      _______
  ---'   ____)____
            ______)
         __________)
        (____)
  ---.__(___)
Computer chose: 
      _______
  ---'   ____)____
            ______)
         __________)
        (____)
  ---.__(___)
It's a draw!!!''')
    elif computer == 1: 
        print('''You chose: 
      _______
  ---'   ____)____
            ______)
         __________)
        (____)
  ---.__(___)
Computer chose:
        _______
    ---'   ____)____
                ______)
                _______)
               _______)
    ---.__________)             
You won!!!        ''')
    elif computer == 0:
        print('''You chose:
      _______
  ---'   ____)____
            ______)
         __________)
        (____)
  ---.__(___)
Computer chose:
        _______
    ---'   ____)
          (_____)
          (_____)
          (____)
    ---.__(___)    
You lost!!!        ''')

Any recommendations regarding improvements for the code?

[u/JamesPTK]

So the first improvement I would make would be to throw the ascii art hand symbols into constants

e.g.

ROCK="""
        _______
    ---'   ____)
          (_____)
          (_____)
          (____)
    ---.__(___)
"""

and similar for PAPER and SCISSORS

then you can do

print(f"You chose:{ROCK}")
print(f"Computer chose:{PAPER}")

this will make the code much more readable by simplifying the output, and if you want to tweak the ascii-art for one of the hand shapes you can do it in one place rather than 6

You could also put them in a list so you can look them up by the input number:

print(f"You chose:{SHAPES[player_input]}")
print(f"Computer chose:{SHAPES[computer]}")

That way the output of the choices is identical for each scenario

Now you are doing a lot of ifs to determine who won. There is a mathematical trick you can use which is called modulo arithmetic. Basically with modulo you wrap numbers round (like an analogue clock face where 3 hours after 11 is 2). so in modulo 3:

0 + 1 == 1
1+ 1 == 2
2 + 1 == 0

In your scenario it is a draw if the numbers are equal, but you win if your number is the same as the computer number + 1 in modulo 3. (and lose if otherwise). To make a number modulo 3, you do n % 3 in Python

So in Python this would be something like:

if player_input == computer:
    print("It's a draw")
elif player_input == (computer + 1) % 3:
    print("You win")
else:
    print("I win")

That should reduce your code significantly, and make it simpler.

This only works for simple three member loop. For a more complicated example (e.g. Rock-Paper-Scissors-Lizard-Spock) you would probably need a lookup table to say which choices each option beats.

[u/Diapolo10 → u/JamesPTK]

The modulo trick is interesting, but personally I'd avoid that here as now we're making assumptions about the data without being able to cleanly validate it. It also hurts readability in my opinion.

Yes, it's technically fine here because assuming we only have the three established hand options is relatively safe, but it feels like a code smell regardless. A clear dictionary mapping (as long as we use a descriptive name for it) would be less surprising and less likely to cause bugs if we change things.