3
\$\begingroup\$

I'm a beginner at coding with Python, and I've only made small projects, and this is one of them. One part of the code is from an answer on Stack Overflow that I made to solve a problem (is the try part) and all the other code is mine.

import random,os,time
score = {'human':0,'robot':0}
def game(): #Basic Game code.
    time.sleep(1)
    os.system('clear')
    print('IA: ' + str(score['robot']) + ' Human: ' + str(score['human']))
    print('1.Rock 2.Scissors 3.Paper')
    while True:
        try:
            choice = int(input("Input your choice:\n"))
            if choice < 1 or choice > 3: #Valid number but outside range, don't let through
                raise ValueError
            else: #Valid number within range, quit loop and the variable selection contains the input.
                break
        except ValueError: #Invalid input
            print("Enter a number from 1 to 3.")
    rchoice = random.randint(1,3)
    time.sleep(1)
    if rchoice == 1:
        print('IA choice is Rock.')
    elif rchoice == 2:
        print('IA choice is Scissors.')
    elif rchoice == 3:
        print('IA choice is Paper.')

    if (rchoice==1 and choice==2)or(rchoice==2 and choice==3)or(rchoice==3 and choice==1):
        print('IA wins!\n')
        score['robot'] += 1
        time.sleep(0.5)
    elif (rchoice == choice):
        print("Draw, let's repeat")
        time.sleep(0.5)
        game()
    else:
        print('You win!\n')
        time.sleep(0.5)
        score['human'] += 1

def startgame(): #The introduction of the game.
    os.system('clear')
    print('Rock-Paper-Scissors v.1')
    print('Developed by P.R.B.\n')
    ngames = int(input('How many games do you want to play?\n'))
    time.sleep(1)
    i = 0
    while i != ngames:
        game()
        i += 1
    print('Final Score:')
    print('IA: ' + str(score['robot']) + ' Human: ' + str(score['human']))
    if score['human'] > score['robot']:
        print('You win the game!\n')
    else:
        print('Game Over')
        print('IA wins the game!\n')

startgame()
\$\endgroup\$
1
\$\begingroup\$

DISCLAIMER: I apologize in advance for the long post. Pick and choose the recommendations I have, especially the part about using clear. Hopefully there aren't any typos/errors in here.

This question is similar to another question I responded to earlier (located here), and will be using roughly the same comments below.

Imports

As noted by PEP8 and personal experience, it's easier to keep track of imports by having them on separate lines, e.g.

import random
import os
import time

Also, since you're only using specific things from each module, you can simply import each separate you're interested in:

from random import randint
from os import system
from time import sleep

Lastly, judging by the way you're using os, you're probably better off using subprocess instead. It's supposed to be an improved version.

So, instead of:

import os
# ...
os.system('clear')

You can do:

from subprocess import call
# ...
call(['clear'])

Usage of clear

If you're certain any OS that is running your code will have a clear command, skip to the next section.

I like how you use clear within the game.

Unfortunately, your code isn't cross-platform (mainly because clear doesn't exist on windows, rather its cls). Whether that matters to you or not, it's be best to check which OS you are currently on.

I'm not really sure what the best route to go with this would be.

This was my way of supporting Windows:

from sys import platform

# use __ to denote to others not to modify this variable
if platform == "win32":
    __clear_cmd = 'cls'
else: # assume any other OS uses "clear"
    __clear_cmd = 'clear'

Whenever you call the clear command, you'll have to do something such as:

call([__clear_cmd], shell=True)

Here for more information about this.

Unnecessary Comments

A lot of your code is self-documenting already and do not require comments (which is a good thing).

You should only comment areas that you believe might be hard to understand.

Variable Names

I'd suggest a rename of a few identifiers, namely:

game:

  • game to play_rps (or something more descriptive)
  • choice to user_choice
  • rchoice to robot_choice (to stay consistent with your name in score)

startgame:

  • startgame to game_counter (or something more descriptive)
  • ngames to num_games

Outputs:

  • IA to Robot (to stay consistent)
  • Human to Player (to stay consistent)

User Input

As noted by your usage of score, I'm assuming you're aware of how dictionaries work.

Just like how you kept track of the human and robot score, you can use a dictionary to keep track of valid user input:

options = {1:'rock', 2:'paper', 3:'scissors'} 

Using options and a slight rearrangement of code within your while loop, we can produce the following:

while choice not in options:
    try:
        choice = int(input("Input your choice: "))
        if user_choice not in options:
            raise ValueError
    except ValueError:
        print("Enter a number from 1 to 3.")

Computer Choice

Adding the options to our code (from the previous section) simplifies the later sections:

print('IA chose:', options[robot_choice])

Calculating the Winner

This part is debatable. I'd rather have the lines written out as such:

winner = None
#...
# determine winner of current game
if user_choice == robot_choice:
    winner = None
elif user_choice == 1: # rock
    if robot_choice == 2:
        winner = 'robot'
    else:
        winner = 'player'
elif user_choice == 2: # paper
    if robot_choice == 3:
        winner = 'robot'
    else:
        winner = 'player'
else: # scissors
    if robot_choice == 1:
        winner = 'robot'
    else:
        winner = 'player'

# update score of winner if applicable
if winner:
    print(winner, 'won!')
else:
    print('Draw, let\'s repeat')

Your code has a (very, very slim chance) of getting a stack overflow, but still is possible.

If you keep getting a draw, your program's call stack is going to look something along the lines of:

start_game():
    game():
        game():
            game():
                ...and so on.

Avoid this by returning a value, marking whether or not the game completed.

# update score of winner if applicable
if winner:
    print(winner, 'won!')
else:
    print('Draw, let\'s repeat')

# returns None if no winner
return winner

startgame

Your num_games while loop can encounter the following errors:

  1. It can infinitely run if the user enters a negative amount of games to play
  2. Crash if the user inputs a value (that isn't an integer)

Depending on how you wish to do this (loop or not), perform a try/except to catch this (just like how you did user_choice):

num_games = 0
try:
    num_games = int(input('How many games do you want to play?\n'))
except ValueError:
    print('Invalid amount of games to play! Quitting game.')
    num_games = 0

Another issue: You aren't checking to see what happens if the player and robot score are the same. Add an == case to this.

Move score into this method. We can use the winner returned from the other function to see who won, and control how many games we wish to play.

Use a for loop, mixed with a while loop, to keep track of games played and score (_ is common to note an unused variable):

winner = None
for _ in range(num_games):
    # play current game until there is a winner
    while not winner:
        winner = play_rps()

    # update score of winner
    score[winner] += 1

Lastly, when you call game_counter (or startgame in your original code), you should make sure your code only runs if it's called as main, e.g.

if __name__ == '__main__':
    game_counter()

and being called via python rps.py or however the file name hosting your code may be.

More info on __main__ here.

End Product

After applying all the above recommendations and some extra tweaks, here's the resulting code (adding cross-platform support command made things a bit annoying):

from random import randint
from subprocess import call
from sys import platform
from time import sleep

if platform == "win32":
    __clear_cmd = 'cls'
else: # assume every other OS uses "clear"
    __clear_cmd = 'clear'


def play_rps():
    sleep(1)

    options = {1:'Rock', 2:'Paper', 3:'Scissors'}
    user_choice = None

    while user_choice not in options:
        print('1. Rock')
        print('2. Scissors')
        print('3. Paper')

        try:
            user_choice = int(input("Input your choice: "))

            if user_choice not in options:
                raise ValueError
        except ValueError: #Invalid input
            print("ERROR: Input must be a number within 1 to 3.")

    # determine Robot's choice
    robot_choice = randint(1,3)
    sleep(1)
    print('Robot choice is:', options[robot_choice])

    # determine winner of current game
    if user_choice == robot_choice:
        winner = None
    elif user_choice == 1: # rock
        if robot_choice == 2:
            winner = 'Robot'
        else:
            winner = 'Player'
    elif user_choice == 2: # paper
        if robot_choice == 3:
            winner = 'Robot'
        else:
            winner = 'Player'
    else: # scissors
        if robot_choice == 1:
            winner = 'Robot'
        else:
            winner = 'Player'

    # update score of winner if applicable
    if winner:
        print(winner, 'won!')
    else:
        print('Draw, let\'s repeat')

    sleep(0.5)

    return winner

def game_counter():
    call([__clear_cmd], shell=True)

    print('Rock-Paper-Scissors v.1')
    print('Developed by P.R.B.\n')

    score = {'Player':0,'Robot':0}
    winner = None
    num_games = 0

    try:
        num_games = int(input('How many games do you want to play?\n'))

        if num_games <= 0:
            raise ValueError
    except ValueError:
        print('Invalid amount of games to play! Quitting game.')
        num_games = 0

    sleep(1)

    for _ in range(num_games):
        # play current game until there is a winner
        while not winner:
            sleep(1)
            call([__clear_cmd], shell=True)
            print('Robot:', score['Robot'], 'Player: ', score['Player'])
            winner = play_rps()

        # update score after game and reset winner
        score[winner] += 1
        winner = None

    # played at least one game
    if num_games > 0:
        print('Final Score:')
        print('Robot:', score['Robot'], 'Player: ', score['Player'])
        if score['Player'] > score['Robot']:
            print('You won the game!\n')
        elif score['Player'] == score['Robot']:
            print('You both tied.\n')
        else:
            print('Robot won the game!\n')

if __name__ == '__main__':
    game_counter()
\$\endgroup\$
3
\$\begingroup\$

It looks like you are familiar with dictionaries and maybe lists. So, let's look at how you collect and enter input. It looks like you are trying to control erroneous input with your try/except and if statement. There is some trouble though. I can send a float through your code if it has a value like 1.2 or 2.5. Your code will turn it into an int and then try to use the value, but there's another way to handle something like this:

if choice not in ['1','2','3']:
    raise ValueError
else:
    break

So, in this case, I'm using a list of the only acceptable values, checking if the choice is in there, and then moving forward.

Your control flow looks a little messy. There are a couple of ways to handle this.

If we keep the same general approach, you can make it more readable by spreading it out. I would actually make a new function for this:

def compare(x,y):
    if x == y:
        state = "It's a Draw!"
    if x == '1':
        if y == '2':
            state = "You Lose!"
        else:
            state == "You Win!"
    elif x == '2':
        if y == '1':
            state = "You Win!"
        else:
            state == "You Lose!"
    else:
        if y == '1':
            state = "You Lose!"
        elif y == '2':
            state = "You Win!"
    return state

However, depending on how comfortable you are with lists, there is another, simpler way!

You can setup a list of the possible game conditions and then do whatever you want to them:

game_result = ["tie!", "win!", "lose!"]

(Actually, that sequence in the list was coded for 1) Rock, 2)Paper, and 3)Scissors. There should be another arrangement for rock-scissors-paper though that makes things smooth).

Then define a comparison function that just looks at the modulated difference between the user and robot input (you'll need to convert them to the int data type first).

def compare(x,y):
result = game_result[ (x-y)%3 ]
return result

There's a few tricky things with how you're checking which round you should be on. So, first thing, it might be helpful/more readable if you change the variable name to something useful like round_number or matches_left.

Then, you have it set to end the while loop if the round number is not equal to n. There's another potential bug here. What if the user enters a float or something you don't want? -- Actually, you're using int() on the input, which corrects for this. So good call! But, that design pattern doesn't always carry over well and it may not be as clear to other readers ( it wasn't to me anyways).

Suppose the player enters 1.2, then your while loop goes on for ever if you aren't using int(). One way to handle this is to use try/except; another way is to use isinstance; and another way is to do something like this:

while num_of_rounds <= ngames:
        game()
        num_of_rounds += 1

In this way, you cover a broader set of what is acceptable/unacceptable and don't have your code hinging on a small window of operation (and exploitability).

Overall, your code works and was fun to play! The biggest things are style, readability, and different ideas on how to handle incorrect input. I would check out the PEP 8 Python style Guide for some general insights: https://www.python.org/dev/peps/pep-0008/

\$\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.