3
\$\begingroup\$

I tried to learn Python during the weekend and after getting some knowledge decided to make a small rock-paper-scissors game. I would be grateful for your input on how I could improve my code. All feedback is highly appreciated.

import random


def round_winner(choice):
    ai_chosen = str(random.randint(1, 3))
    print(f'AI chose {ai_chosen}')

    if choice == '1' and ai_chosen == '2':
        return 'ai'
    elif choice == '2' and ai_chosen == '3':
        return 'ai'
    elif choice == '3' and ai_chosen == '1':
        return 'ai'
    elif choice == ai_chosen:
        return 'tie'
    else:
        return 'player'


def display_round_winner(winner):
    if winner == 'tie':
        print('This round is tied!')
    else:
        print(f'The winner this round is the {winner.upper()}')

    print(f'''
    Current points as follows:
    Player: {counter['player']}
    AI: {counter['ai']}
    Rounds Tied: {counter['tie']}
    ''')


def score_checker():
    global game_ongoing
    for key, value in counter.items():
        if value == 2:
            print(f'{key.upper()} wins the game!')
            game_ongoing = False


def initializer():
    global counter
    message = '''
    Please choose one of the following:
    1: Rock
    2: Paper
    3: Scissors
    '''

    print(message)

    choice_of_obj = input('What will it be: ')
    if choice_of_obj in ['1', '2', '3']:
        winner = round_winner(choice_of_obj)
        counter[winner] += 1
        display_round_winner(winner)
        score_checker()
    else:
        print('Out of bounds')


counter = {
    'player': 0,
    'ai': 0,
    'tie': 0
}

game_ongoing = True

while game_ongoing:
    initializer()
\$\endgroup\$
3
\$\begingroup\$

You ask the player to enter 1, 2 or 3 for Rock, Paper or Scissors. When you tell the player what the AI choose, you say it was a 1, 2 or a 3. It would be friendlier if you said what they choose. You could do this with a dictionary that translates the abbreviated choice into the actual item.

choices = { '1': 'Rock', '2': 'Paper', '3': 'Scissors' }

def round_winner(choice):
    ai_chosen = ...
    print(f'AI chose {choice[ai_chosen]}')
    ...

Also, you could use that dictionary to print out the menu for the player, instead of hard-coding it:

    print('Please choose one of the following:')
    for choice, item in choices:
        print(f'{choice}: {item}')

You are using “ai”, “player”, and “tie” as keys for your counter dictionary, and always printing out winner.upper() when you print out a winner. You could just use “AI”, “PLAYER” and “TIE” as the dictionary keys, avoiding the need for the .upper() calls.


score_checker is an odd name. Perhaps one of the hardest things about programming is coming up with good names. check_for_game_winner might be better.

Using global is almost always bad. You just need to pass a true/false value back to the caller to indicate if the game is over. Use a return statement. Ie, inside if value == 2:, add a return True statement.


initializer is another terrible name. play_round would be better.

Checking for the overall winner inside play_round is confusing responsibilities. The play_round function doesn’t know it is being called in a loop, if at all. It should be removed from here.

global counter is again a bad idea. You could simply pass the counter in as an argument.


Instead of having the game code run directly, you should add a play_games function, and move the counter initialization code and loop inside that. With other changes, above, it might look like:

def play_games():
    counter = { 'PLAYER': 0, 'AI':0, 'TIE': 0}
    while True:
        play_round(counter)
        if check_for_game_winner(counter):
            break

The file should only execute code if the file is the main program. If the file is imported into another file, you wouldn’t want the code to automatically run. The following guard is usually used for this:

if __name__ == '__main__':
    play_game()

Your model of storing the player’s & AI’s moves as strings is perhaps not the best. If you used integers, you could perform the rock beats scissors beats paper beats rock test with modulo arithmetic:

if ai_choice % 3  == (choice + 1) % 3:
    # the AI won
\$\endgroup\$
  • \$\begingroup\$ Thanks for the input! c: \$\endgroup\$ – aleisley Mar 19 at 4:36

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.