patternpythonModerate
A Simple Rock, Paper, Scissors program
Viewed 0 times
scissorspapersimpleprogramrock
Problem
I wrote a rock, paper, scissors program. It works fine, but I'm curious if there are any constructive critiques, tips, tricks, comments, or advice anyone may have on how the code operates, performs, looks, etc.
# This code is a PvC Rock-Paper-Scissors Game!
# Import Random Library for Computer Choice
import random
# Greeting & Introduction
print ("Hello! Let's play Rock-Paper-Scissors!");
# Just to Make things Smoother.
options = {1:'rock', 2:'paper', 3:'scissors'};
player_result = ["tie!", "win!", "lose!"];
# Obtain Player & CPU Inputs
def collection(user_choice = 42):
# user_choice Collection
while (user_choice not in ['1','2','3']):
# Prompts the User for a number & Stores as raw_input
print ("Pick a Number: \n 1) \t Rock \n 2) \t Scissors \n 3) \t Paper \n");
user_choice = raw_input();
# Checks to see if the choice is acceptable
if (user_choice in ['1','2','3']):
break;
# If not acceptable, restart the loop (ask again).
else:
print("\nPlease pick either 1, 2, or 3!\n");
# Convert user_choice to Int
user_choice = int(user_choice);
# choose comp_choice
comp_choice = random.randint(1,3);
return (user_choice, comp_choice);
# Translate Results into English
def translate_choices(user,comp):
print ("\nYou chose " + options[user] +"!");
print ("Computer chose " + options[comp] +"!");
# Comparison Function returns Game Results
def compare(x,y):
result = player_result[ (x-y)%3 ];
return result;
(user_choice,comp_choice) = collection();
translate_choices(user_choice,comp_choice);
result = compare (user_choice, comp_choice);
print ("You " + result);Solution
From the top:
Imports
Since you're only using one thing from
As PEP8 recommends, whenever the time comes order your imports by standard imports, then third-party imports, then lastly local imports.
Use of Parentheses
In Python 2, your
As a later note: You can use commas within print statements to have spaces between things or you can concatenate two strings with the
I wouldn't mix the two though (might make things look messy), and this is just a helpful note on the difference.
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, e.g. calculation for the game result.
If you end up using functions, you might want to add some helpful docstrings to describe what's going on, or rename the function e.g.
Code Organization
A lot of your code doesn't seem to need to be in functions, e.g.
This is a bit of personal preference, but it might be nicer to move all your code into a function and avoid having global variables. This way, if others end up importing your code they won't encounter conflicts.
Also, it might be good to make sure your code only runs if it's called as main, e.g.
and being called via
More info on
Handling User Choice
Instead of making a list every time, why not use the keys from the dictionary you already made?
Also, to make the logic a bit simpler in the while loop, you can remove the break statement by only checking for invalid inputs:
Lastly, you can also merge the later conversion of
Unnecessary Semicolons
Do you come from a C++/Java background? Reason I ask is because you seem to have a lot of semicolons in your code (which aren't required for python).
End Product
With the suggestions I listed above (and a few small things such as splitting up your option prompt), here's the code I came up with:
NOTE: With the above format, it also makes creating a loop (if you choose to have a replay option) much easier.
Imports
Since you're only using one thing from
random, you might as well only import that one thing:from random import randintAs PEP8 recommends, whenever the time comes order your imports by standard imports, then third-party imports, then lastly local imports.
Use of Parentheses
In Python 2, your
print statements do not require parentheses. If you're on Python 3, print function calls require them. Other areas (such as your while loop and return statement for collections) do not require parentheses around them.As a later note: You can use commas within print statements to have spaces between things or you can concatenate two strings with the
+ operator, e.g.>>> print "Hello", "World!"
Hello World!
>>> print "Hello" + "World!"
HelloWorld!I wouldn't mix the two though (might make things look messy), and this is just a helpful note on the difference.
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, e.g. calculation for the game result.
If you end up using functions, you might want to add some helpful docstrings to describe what's going on, or rename the function e.g.
def calculate_winner(x, y):
"""."""
#...etc...Code Organization
A lot of your code doesn't seem to need to be in functions, e.g.
translate_choices and compare. You only ever use them once, and its all related to your game.This is a bit of personal preference, but it might be nicer to move all your code into a function and avoid having global variables. This way, if others end up importing your code they won't encounter conflicts.
def play_rps():
# ... rest of your code here ...Also, it might be good to make sure your code only runs if it's called as main, e.g.
if __name__ == '__main__':
play_rps()and being called via
python rps_name.py or however the file name hosting your code may be.More info on
__main__ here.Handling User Choice
Instead of making a list every time, why not use the keys from the dictionary you already made?
while user_choice not in options:
# do user_choice stuff hereAlso, to make the logic a bit simpler in the while loop, you can remove the break statement by only checking for invalid inputs:
while user_choice not in options:
#...print prompt for user choice here...
user_choice = raw_input()
if user_choice not in options:
print '\nPlease pick either 1, 2, or 3!\n'Lastly, you can also merge the later conversion of
raw_input into an integer into your while loop:try:
user_choice = int(raw_input())
except ValueError:
# input was not a valid integer
user_choice = None
if user_choice not in options:
print '\nPlease pick either 1, 2, or 3!\n'Unnecessary Semicolons
Do you come from a C++/Java background? Reason I ask is because you seem to have a lot of semicolons in your code (which aren't required for python).
End Product
With the suggestions I listed above (and a few small things such as splitting up your option prompt), here's the code I came up with:
from random import randint
def play_rps():
print "Hello! Let\'s play Rock-Paper-Scissors!\n"
options = {1:"rock", 2:"paper", 3:"scissors"}
player_result = ["tie", "win", "lose"]
user_choice = None
# get user choice
while user_choice not in options:
print "Game options:"
print "-------------"
print "1) Rock"
print "2) Scissors"
print "3) Paper"
try:
user_choice = int(raw_input("Your choice: "))
except ValueError:
# input was not a valid integer
user_choice = None
if user_choice not in options:
print "\nPlease pick either 1, 2, or 3!\n"
# calculate computer choice
computer_choice = randint(1, 3)
print "\nYou chose: " + options[user_choice] + "!"
print 'Computer chose: ' + options[computer_choice] + "!"
# calculate winner
result = player_result[ (user_choice-computer_choice) % 3]
print 'You ' + result + '!'
if __name__ == '__main__':
run_game = play_rps()NOTE: With the above format, it also makes creating a loop (if you choose to have a replay option) much easier.
Code Snippets
from random import randint>>> print "Hello", "World!"
Hello World!
>>> print "Hello" + "World!"
HelloWorld!def calculate_winner(x, y):
"""<Explain how calculate_winner works here>."""
#...etc...def play_rps():
# ... rest of your code here ...if __name__ == '__main__':
play_rps()Context
StackExchange Code Review Q#157607, answer score: 13
Revisions (0)
No revisions yet.