3
\$\begingroup\$

It's been a while since I've done any sort of programming, and my first time in Python. I'm just looking for any general feedback and tips since this is my first program.

# rock, paper, scissors game

import random
playerWin = 0
aiWin = 0
guess = 0
print('You are playing rock, paper, scissors. If you would like to quit, type "4"')

while int(guess) != 4:
    choices = ['', 'rock', 'paper', 'scissors']
    aiGuess = random.randint(1, 3)

    # data validation
    while True:
        guess = input('Enter "1" for rock, "2" for paper, or "3" for scissors. "4" to quit: ')
        if str(guess) in ('1', '2', '3', '4'):
            break
        else:
            print('ERROR: NOT AN OPTION. TRY AGAIN.\n')

    # check player guess vs AI guess
    if int(guess) == aiGuess:  # Tie game
        print('Tie game. The AI chose ' + choices[aiGuess] + '.')
    elif (int(guess) == 1 and aiGuess == 3) or (int(guess) == 2 and aiGuess == 1) or (
            int(guess) == 3 and aiGuess == 2):  # player wins
        print('You win! The AI chose ' + choices[aiGuess] + '.')
        playerWin += 1
    elif int(guess) == 4:  # quit break
        break
    else:  # AI wins
        print('You lost. The AI chose ' + choices[aiGuess] + '.')
        aiWin += 1

    print('SCORE: You have ' + str(playerWin) + ' points and the AI has ' + str(aiWin) + ' points.\n')

# final score print
print('\n################################################################')
print('FINAL SCORE: You ended with ' + str(playerWin) + ' points and the AI had ' + str(aiWin) + ' points.')
print('################################################################')
\$\endgroup\$
4
\$\begingroup\$

First of all, great work! The program appears solid, the interface is decent and you handle bad input.

I have a few suggestions for improvement:

UI/UX

  • The "1" "2" "3" "4" approach for input is nice in that the keys are near each other, but maybe "r" "p" "s" and "q" would make more semantic sense. You could generalize the input to allow "r" or full words like "rock" to be more flexible.
  • After a while, Enter "1" for rock, "2" for paper, or "3" for scissors. "4" to quit: gets a little verbose and distracting, so you might want to abbreviate this after a few rounds, or have a special "help" command for instructions. Maybe show the reminder only after bad input was received.
  • ERROR: NOT AN OPTION. TRY AGAIN. is grating due to the caps. I prefer my apps to be gentle on me when I mess up, especially if I'm enjoying a relaxing game of RPS.
  • Sending a Ctrl+C to kill the app raises a KeyboardError. You could catch this and show final scores and exit cleanly.
  • A "play again?" prompt, setting a target goal score, or allowing two humans to play would be nice.

Minor issues

  • Per PEP-8, always use snake_case, not camelCase in Python.

  • Add a line or two of vertical whitespace after imports. Check out black which will format your code nicely.

  • int(guess) happens 6 times and is rather hard on the eyes. If you even need to cast (you could just use strings throughout), I'd prefer to do it once up-front.

  • The code for the "AI":

    choices = ['', 'rock', 'paper', 'scissors']
    aiGuess = random.randint(1, 3)
    

    could be:

    choices = ["rock", "paper", "scissors"]
    ai_guess = random.randint(0, len(choices))
    

    or better yet:

    ai_guess = random.choice(("rock", "paper", "scissors"))
    
  • In if str(guess) in ('1', '2', '3', '4'):, the str() cast is superfluous since input() always returns a string. It's best to move valid choices out of this hardcoded tuple and up to the top of the script.

  • Use f-strings instead of +:

    print('SCORE: You have ' + str(playerWin) + ' points and the AI has ' + str(aiWin) + ' points.\n')
    

    becomes

    print(f'SCORE: You have {playerWin} points and the AI has {aiWin} points.\n')
    
  • \n################################################################' could be "\n" + "#" * 64.

Design

If your goal is to get the code working as directly as possible, then plopping everything into one function in the global scope is fine. But for larger apps, you'll want to think through the design a bit more to make sure it's maintainable and extensible. For example, you might want to add lizard and spock, a GUI, multiplayer over the network, or other features.

A more maintainable design would be to separate user interaction and game logic and generalize where appropriate. Using functions is a good way to achieve this. Your code has comments delimiting sections like # data validation, # check player guess vs AI guess and # final score print. These are good candidates for functions; the comments are a crutch here.

Additionally, the code has high cyclomatic complexity (nested and wide branches and loops). Some of the conditions are lengthy and difficult to understand:

elif (int(guess) == 1 and aiGuess == 3) or (int(guess) == 2 and aiGuess == 1) or (
        int(guess) == 3 and aiGuess == 2):  # player wins
    ...

This would be another great refactoring opportunity; a function could help out quite a bit here, with the goal something like if player_wins(player_guess, ai_guess): ... from the client perspective.

Dictionaries are a good way of encoding mappings between entities. For example, in RPS we have relationships between each of the items. By creating a dictionary of "X beats Y" relationships, we can avoid enumerating every possibility of the options and get an instant look-up to find out whether an item beats another.

Since an RPS game is stateful and control flow passes back and forth between the UI and game engine, a class or two might be a reasonable design decision.

Suggested rewrite

import random


class RockPaperScissorsGame:
    PLAYER = 0
    COMPUTER = 1

    def __init__(self, objects):
        if len(objects) < 3:
            raise ArgumentError("There must be at least 3 objects")

        self._objects = objects
        self._player_score = 0
        self._computer_score = 0

    @property
    def objects(self):
        return dict(self._objects)

    @property
    def player_score(self):
        return self._player_score

    @property
    def computer_score(self):
        return self._computer_score

    def validate_choice(self, choice):
        return choice in self._objects

    def handle_round(self, player_choice, computer_choice):
        if self._objects[player_choice] == computer_choice:
            self._player_score += 1
            return self.PLAYER
        elif self._objects[computer_choice] == player_choice:
            self._computer_score += 1
            return self.COMPUTER


class RockPaperScissorsUI:
    def __init__(self, objects):
        self.objects = objects
        self.abbrevations = {x[0]: x for x in objects}

    def play_one_game(self):
        self.game = game = RockPaperScissorsGame(self.objects)
        print(f'You are playing {", ".join(self.objects)}.\n'
               'If you would like to quit, type "quit"\n')
        
        while self.play_one_round():
            print(f"You have {game.player_score} points and "
                  f"the AI has {game.computer_score} points.\n")

        print(f"\n{'#' * 64}\nYou ended with {game.player_score} points and "
              f"the AI had {game.computer_score} points.\n{'#' * 64}")

    def play_one_round(self):
        while True:
            prompt = f'Choose {", ".join(self.objects)}: '
            user_input = input(prompt).lower()
            user_input = self.abbrevations.get(user_input, user_input)

            if user_input in ("q", "quit", "exit"):
                return False
            elif self.game.validate_choice(user_input):
                self.make_move(user_input)
                return True
            else:
                print(f"Error: '{user_input}' is not an option. Try again.\n")

    def make_move(self, user_input):
        computer_choice = random.choice(list(self.objects))
        result = self.game.handle_round(user_input, computer_choice)
        result_message = {
            self.game.PLAYER: "You win!", 
            self.game.COMPUTER: "You lost!"
        }.get(result, "Tie game.")
        print(f"{result_message} The computer chose {computer_choice}.")


if __name__ == "__main__":
    objects = {
        "rock": "scissors",
        "paper": "rock",
        "scissors": "paper",
    }
    game = RockPaperScissorsUI(objects)
    game.play_one_game()

Sample run:

You are playing rock, paper, scissors.
If you would like to quit, type "quit"

Choose rock, paper, scissors: Rock
You lost! The AI chose paper.
You have 0 points and the AI has 1 points.

Choose rock, paper, scissors: rock
You lost! The AI chose paper.
You have 0 points and the AI has 2 points.

Choose rock, paper, scissors: p
You lost! The AI chose scissors.
You have 0 points and the AI has 3 points.

Choose rock, paper, scissors: asdasd
Error: 'asdasd' is not an option. Try again.

Choose rock, paper, scissors: q

################################################################
You ended with 0 points and the AI had 3 points.
################################################################
\$\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.