Rock, Paper, Scissors game made in python

Question

I added Rock, Paper, Scissors to one of my programs. I'm new to python and PEP 8, so I want to see how to improve my work.

def rock_paper_scissors(name):
 play_rps = True
 while play_rps:
 rps = ["rock", 'paper', 'scissors']
 player_rps = input("Rock, Paper, or Scissors: ").lower()
 com_rps = rps[random.randint(0,3)]
 print(com_rps)
 if com_rps == player_rps:
 print('Tie')
 if com_rps == 'rock' and player_rps == "scissors":
 print("Chatter Bot Wins!")
 if com_rps == 'scissors' and player_rps == "paper":
 print("Chatter Bot Wins!")
 if com_rps == 'paper' and player_rps == "rock":
 print("Chatter Bot Wins!")
 if player_rps == 'rock' and com_rps == "scissors":
 print(f"{name} Wins!")
 if player_rps == 'sicssors' and com_rps == "paper":
 print(f"{name} Wins!")
 if player_rps == 'paper' and com_rps == "rock":
 print(f"{name} Wins!")
 yn = input("Do you want to play again. Y/N: ").lower()
 if yn == 'n' or yn == 'no':
 play_rps = False

• GameDungeon: I forgot what pass meant.
• Peilonrayz: Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers .

Answer 1

Here are some extra suggestions:

1. Considering that this is a piece of a much larger system, I'd suggest that you make it a class. In fact, make it two classes: the game, and the input/output mechanism. That should facilitate writing unit tests. I'm not going to assume a class for my remaining suggestions, but the point stands.

2. As @Peilonrayz pointed out, you need some more functions. I'll suggest that you focus on creating functions for general-purpose interaction with the user. This would let you re-use those same functions in other games, other parts of your bot, etc.

   player_rps = input("Rock, Paper, or Scissors: ").lower()
   

   This is a potential bug. What happens if I don't spell my answer right, or if I enter "Gecko" instead? So, write a function to get a choice from the user. Allow abbreviations. Something like:

   def input_choice(choices, default=None):
    """ Presents a list of choices to the user and prompts her to enter one. If 
    default is not None, an empty input chooses the default. Abbreviations are
    allowed if they identify exactly one choice. Case is ignored.
    Returns the chosen value as it appears in the choices list (no abbreviations).
    """
    pass
   

3. Use random.choice to choose a random value from a sequence. You don't need to pick a number and then index with it:

   rps = ["rock", 'paper', 'scissors']
   player_rps = input("Rock, Paper, or Scissors: ").lower()
   com_rps = rps[random.randint(0,3)]
   

   becomes:

   rps = "Rock Paper Scissors".split()
   player_rps = input_choice(rps)
   com_rps = random.choice(rps)
   

4. You can merge your data to simplify your code. There are many paragraphs that look like this:

   if com_rps == 'rock' and player_rps == "scissors":
    print("Chatter Bot Wins!")
   

   This is really a 2-inputs -> 1-output function, but you can use a dictionary for this just by concatenating the inputs:

   i_win = "Chatter Bot wins!"
   u_win = f"{name} wins!"
   winner = {
    "Rock:Scissors": i_win,
    "Scissors:Paper": i_win,
    "Paper:Rock": i_win,
    "Scissors:Rock": u_win,
    "Paper:Scissors": u_win,
    "Rock:Paper": u_win,
   }
   contest = f"{com_rps}:{player_rps}"
   print(winner.get(contest, "It's a tie!"))
   

   Edit:

   @Peilonrayz pointed out that tuples were a better choice than strings, and he's right. So here's a slightly different version:

   winners = { ('Rock', 'Scissors'), ('Scissors', 'Paper'), ('Paper', 'Rock'), }
   result = "Chatter Bot wins!" if (com_rps, player_rps) in winners
    else f"{name} wins!" if (player_rps, com_rps) in winners
    else "It's a tie!"
   print(result)
   

5. You need more functions:

   yn = input("Do you want to play again. Y/N: ").lower()
   if yn == 'n' or yn == 'no':
    play_rps = False
   

   This should be a function, like:

   def input_yn(prompt, default=None):
    """ Prompts the user for a yes/no answer. If default is provided, then an 
    empty response will be considered that value. Returns True for yes, 
    False for no.
    """
    pass
   

• Peilonrayz: It's more standard to use tuples than strings if you need multi-key dictionaries. ("Rock": "Scissors"): i_win. Also if you don't ensure that both com_rps and player_rps are valid then you'll be printing "It's a tie!" incorrectly.
• GameDungeon: This worked after I edited a bit.

Answer 2

1. Move more of your code out of the function. Make a find_winner function.
2. You don't need to add _rps to the end of every variable name. It only makes it harder to read the code.
3. PEP 8 doesn't say which string delimiter to use, but says to stick to one. I use ', you may prefer ".
4. You can use random.choice rather than random.randint to select from a list.

5. There is a bug in your code. I suggest you keep using string literals to a minimum to prevent this.

   player_rps == 'sicssors' and com_rps == "paper"
   

6. You can simplify your yn check by using in.

This in all got me:

WIN_VALUES = {
 'rock': 'scissors',
 'paper': 'rock',
 'scissors': 'paper'
}
def find_winner(choice1, choice2):
 if choice1 == choice2:
 return 0
 if WIN_VALUES[choice1] == choice2:
 return 1
 if WIN_VALUES[choice2] == choice1:
 return 2
def rock_paper_scissors(name):
 play = True
 while play:
 player = input('Rock, Paper, or Scissors: ').lower()
 com = random.choice(list(WIN_VALUES))
 print(com)
 winner = find_winner(com, player)
 if winner == 0:
 print('Tie')
 elif winner == 1:
 print('Chatter Bot Wins!')
 elif winner == 2:
 print(f'{name} Wins!')
 else:
 print(f'Invalid input {player}')
 yn = input('Do you want to play again. Y/N: ').lower()
 if yn in ('n', 'no'):
 play = False

• GameDungeon: The reason for the _rps is this is a small chunk of a muck bigger piece of code. That is also why I try'd to keep it in one function, but you helped a lot so thanks.
• Peilonrayz: @hackerHD Even so my comment about it still holds. The only reason I can think you'd do this is if you don't have a main function with your main code in it. If that's the case you should make a main function.
• GameDungeon: Here is a link to the full ChatterBot. codereview.stackexchange.com/questions/213741/…
• aghast: It's worth pointing out that variables appearing only inside the function don't interfere with other variables. So the names don't need the _rps suffix.
• Peilonrayz: @AustinHastings You're correct, I'm unsure why I didn't think that was the case for global variables.