Rock-Paper-Scissors implementation

Question

I built a simple Rock Paper Scissors game in Python as my first "project", and I want a feedback from you. I want your opinion on how I write my code, is it okay and readable enough? PEP8 laws, things to improve or preserve?

Computer Choice - Generating a random choice with the random module and the random.randomchoice() method from a list.

User Choice - Taking an Input from the user as a character (R, P and S) and uppercasing the character.

Finally, I'm checking who is the winner with if conditions.

import random

choices = ['Rock', 'Paper', 'Scissors']
bot_choices = random.SystemRandom().choice(choices)

def result(user, bot):
    return f'\nComputer Choice: {bot}\nYour Choice: {user}'

class RPS:
    def __init__(self):
        self.user_choice = str(input('(R)ock, (P)aper, (S)cissors: ')).upper()
        if not self.user_choice or self.user_choice not in ['R', 'P', 'S']:
            print('Invalid Input!')
        else:
            self.get_winner()
            print(result(self.user_choice, bot_choices))

    def get_winner(self):
        if self.user_choice == 'R' and bot_choices == 'Paper' or self.user_choice == 'P' and bot_choices == 'Scissors' or self.user_choice == 'S' and bot_choices == 'Rock':
            print('Result: Computer Won!')
        elif self.user_choice == 'R' and bot_choices == 'Rock' or self.user_choice == 'P' and bot_choices == 'Paper' or self.user_choice == 'S' and bot_choices == 'Scissors':
            print('Result: Tie!')
        else:
            print('Result: You Won!')

if __name__ == '__main__':
    game = RPS()

Answer 1

Echoing all of VincentRG's points, with a little clarification and demonstration:

1. The only reason to wrap code in a if __name__ == '__main__' block is to keep it from being executed when your module is imported by another module. By that token, the fact that you initialize bot_choices outside of that block means that those random choices will be determined only once at the time of import, so the importing module will get the same result every time it calls RPS(). This is probably not what you want. It doesn't affect the way your program runs since you don't actually have any other modules, but if you're going to check for __main__ you should understand why you're doing that and apply the same logic to other parts of your code.

2. This does not need to be a class and therefore should not be. Here's how to write it as a single simple function (this is mostly just taking your existing code and moving it around):

import random

def rock_paper_scissors() -> None:
    bot_choice = random.SystemRandom().choice(['Rock', 'Paper', 'Scissors'])

    user_choice = str(input('(R)ock, (P)aper, (S)cissors: ')).upper()
    if user_choice not in ['R', 'P', 'S']:
        print('Invalid Input!')
        return

    if (user_choice == 'R' and bot_choice == 'Paper' 
            or user_choice == 'P' and bot_choice == 'Scissors' 
            or user_choice == 'S' and bot_choice == 'Rock'):
        print('Result: Computer Won!')
    elif user_choice == bot_choice[0]:
        print('Result: Tie!')
    else:
        print('Result: You Won!')

    print(f'\nComputer Choice: {bot_choice}\nYour Choice: {user_choice}')

if __name__ == '__main__':
    rock_paper_scissors()

More notes:

• Not necessary to check user_choice for truthiness if you're already checking it for membership in a collection of truthy values.
• The name bot_choices is confusing if it's a single choice. Changed it to bot_choice.
• There's no reason to name the choices variable since you never use it again after picking bot_choice.
• You can simplify the tie check way down by just checking the user choice against the first letter of the bot choice.
• This would be simpler yet if you defined the choices as an Enum and had the player and bot use the same enum instead of their own hardcoded strings.

If you have no need to reuse this function or this module, this script doesn't even need to define a function. You could have a file that just looks like:

import random

bot_choice = random.SystemRandom().choice(['Rock', 'Paper', 'Scissors'])

user_choice = str(input('(R)ock, (P)aper, (S)cissors: ')).upper()
if user_choice not in ['R', 'P', 'S']:
    print('Invalid Input!')
    exit()

if (user_choice == 'R' and bot_choice == 'Paper' 
        or user_choice == 'P' and bot_choice == 'Scissors' 
        or user_choice == 'S' and bot_choice == 'Rock'):
    print('Result: Computer Won!')
elif user_choice == bot_choice[0]:
    print('Result: Tie!')
else:
    print('Result: You Won!')

print(f'\nComputer Choice: {bot_choice}\nYour Choice: {user_choice}')

and it would behave exactly the same.

Here's how you might use an Enum to specify the choices:

import enum
import random

class Choice(enum.Enum):
    ROCK = "R"
    PAPER = "P"
    SCISSORS = "S"

what_beats = {
    Choice.SCISSORS: Choice.ROCK,
    Choice.ROCK: Choice.PAPER,
    Choice.PAPER: Choice.SCISSORS,
}

bot_choice = random.choice([c for c in Choice])

try:
    user_choice = Choice(input('(R)ock, (P)aper, (S)cissors: ').upper())
except ValueError:
    print('Invalid input!')
    exit()

if what_beats[user_choice] == bot_choice:
    print('Result: Computer Won!')
elif user_choice == bot_choice:
    print('Result: Tie!')
else:
    print('Result: You Won!')

print(f'\nComputer Choice: {bot_choice.name.title()}'
      f'\nYour Choice: {user_choice.name.title()}')

Answer 2

Well, there are few points to address.

• Your code is structured in a lot of ways: at the same level, we have variables, a function and a class. You have game logic directly in the module, which is evaluated once (at module import), so your bot always have the same choice made.

• Also, the RPS class is not necessary. It only do stuff on instantiation, and it uses a function outside itself to print each player's choice. I think you should remove it, mainly because you will never need several RPS objects at once, nor have to store informations for a while.

• Regarding to formatting your code, personnaly I always set a character limit per line of 100. I find it helps to keep the code a little bit more readable (personnal choice).