I wrote a simple Rock, Paper Scissors game in Python for class. As a beginner, I would like some input as to how I could abstract or make my code smoother. I feel like the repeated use of if
or elif
statements are overbearing. I would like any tips or advice, or perhaps a different method of approach.
import random
user = ''
while user != 'q':
print('Enter your choice')
print('R or r for rock\nP or p for paper\nS or s for scissors\nQ or q to quit')
user = input('')
user = user.lower()
if user == 'q':
break
comp = random.randrange(1, 4)
choice = comp
# assign (int) to user choices
if user == 'r':
user = 1
elif user == 'p':
user = 2
elif user == 's':
user = 3
# assign string to computer random choice
if comp == 1:
choice = 'rock'
elif comp == 2:
choice = 'paper'
elif comp == 3:
choice = 'scissors'
print('Computer picked:', choice)
# compare selection to figure out the winner
if user == 1 and comp == 3:
print('You win against the computer\n')
elif user == 3 and comp == 1:
print('You lose against the computer\n')
elif user > comp:
print('You win against the computer\n')
elif user < comp:
print('You lose against the computer\n')
elif user == comp:
print('You tie against the computer\n')
-
\$\begingroup\$ What about 'switch case'? \$\endgroup\$the scion– the scion2017年02月17日 07:29:35 +00:00Commented Feb 17, 2017 at 7:29
-
4\$\begingroup\$ @thescion: As this is Python, there is no switch case. That said, RPS is trivially coded up with a dictionary. \$\endgroup\$Martijn Pieters– Martijn Pieters2017年02月17日 07:31:14 +00:00Commented Feb 17, 2017 at 7:31
-
2\$\begingroup\$ We have a rock-paper-sciccors tag. Feel free to browse those questions for alternative implementations. There should be a couple in python as well. \$\endgroup\$Mast– Mast ♦2017年02月17日 07:57:57 +00:00Commented Feb 17, 2017 at 7:57
-
2\$\begingroup\$ Your title should indicate what the code is intended to do - please read our How to Ask, because it's quite different to that of Stack Overflow. \$\endgroup\$Toby Speight– Toby Speight2017年02月17日 09:02:41 +00:00Commented Feb 17, 2017 at 9:02
-
1\$\begingroup\$ I don't think scissors made of paper would be very effective. \$\endgroup\$TylerH– TylerH2017年02月17日 16:09:09 +00:00Commented Feb 17, 2017 at 16:09
6 Answers 6
The way you are trying to solve this problem does not seem pythonic. Since switch
statements don't exist in python, you can simulate switch
-like behaviour using dictionaries.
For example, instead of using if
-else
to set the value of user
, you can so something like
user_options = {
'r': 1,
'p': 2,
's': 3
}
user = user_options[user]
Since functions and classes are first-class objects in python, if you want to accomplish some less trivial, you can use them too:
def func1():
# do some operation
def func2():
# some other operation
def default_func():
# some default action
options = {
'1': func1,
'2': func2,
'default': default_func
}
value = raw_input('enter a number')
result = options.get(int(value), options.get('default'))()
If your functions are simple enough, you can use lambdas as values, which would definitely be more concise.
-
2\$\begingroup\$ What is
user = user_options[user]
doing? You define the dictionary first and then setuser
equal to the corresponding value in the dictionary? \$\endgroup\$Richard Peña– Richard Peña2017年02月17日 07:55:40 +00:00Commented Feb 17, 2017 at 7:55 -
1\$\begingroup\$ In your code, you are setting user =1 if it is equal to 'r', that's exactly what that statement is doing \$\endgroup\$hspandher– hspandher2017年02月17日 07:57:58 +00:00Commented Feb 17, 2017 at 7:57
-
1\$\begingroup\$ Your code won't work.
options[int('1')]
won't lookup on'1'
. \$\endgroup\$wizzwizz4– wizzwizz42017年02月18日 09:32:13 +00:00Commented Feb 18, 2017 at 9:32 -
\$\begingroup\$ You are right. One has to convert into an integer first. Anyway I was just giving him only a general idea on how to approach toward solution. \$\endgroup\$hspandher– hspandher2017年02月18日 12:01:19 +00:00Commented Feb 18, 2017 at 12:01
In Python you can compare multiple values the following way:
if (user, comp) in {(0, 2), (1, 0), (2, 1)} :
print('You win against the computer\n')
elif user == comp :
print('You tie against the computer\n')
else :
print('You lose against the computer\n')
-
\$\begingroup\$ Why do you have curly braces instead of square brackets around the list of win-conditions? That makes it a
dict
without values but only keys? \$\endgroup\$Martin Ueding– Martin Ueding2017年02月18日 09:39:09 +00:00Commented Feb 18, 2017 at 9:39 -
-
\$\begingroup\$ Cool, I did not know that. Seems to be added in Python 3.1, so it is safe to use on all my targeted platforms. Lookup in the set will be faster, so that is a neater solution; though it won't matter with three elements. \$\endgroup\$Martin Ueding– Martin Ueding2017年02月18日 11:37:10 +00:00Commented Feb 18, 2017 at 11:37
-
1\$\begingroup\$ That makes a
Set
. Technically it is indeed a dictionary with only keys. This structure from Python 2, since Python 2.7,{}
can be used to declare a set \$\endgroup\$Zefick– Zefick2017年02月18日 13:30:10 +00:00Commented Feb 18, 2017 at 13:30
Here are 3 corrections. Instead of manually converting r,s,p
and 1,2,3
, use a dictionary. Instead of using a random number for the computers choice, use the players options. Instead of having extra win/lose/tie
checks, have one per outcome.
Fix Conversion
This is a simple way to convert r,p,s
into a name or a value:
options['r']['name'] == 'Rock'
options['r']['value'] == 1
options = {
'r':{'name':'Rock','value':1},
'p':{'name':'Paper','value':2},
's':{'name':'Scissors','value':3}
}
Fix Computer's Choice
By choosing a random key from options prevents having to hard-code values. It also allows us to lookup the name and value for the computer's choice. compChoice
will be r, p, or s
since those are options' keys.
compChoice = random.choice(list(options.keys()))
Fix win/lose/tie
Checks
A tie only happens if compValue==userValue
. Now consider +1 mod 3, for any value if you "+1 mod 3" you will get the only value that can beat it:
rocVal+1 %3 == papVal
papVal+1 %3 == sciVal
sciVal+1 %3 == rocVal
compVal = options[compChoice]['value']
userVal = options[userChoice]['value']
result = 'win'
if userVal == compVal: result = 'tie'
elif userVal+1 % 3 == compVal: result = 'lose'
# else: result = 'win'
print('You '+result+' against the computer!\n')
Advanced Check
But what if you want to play Rock-Paper-Scissors-Lizard-Spock? Or a any version with more than 3 options? If that is the case, here is a scalable solution from sch. Note that the number of options should always be odd. This way each element has the same number of superiors and inferiors.
Explanation: the key piece here is the decider. Picture these options as being on a circle (e.g. an analog clock). decider is the clockwise distance from b
to a
. [Note: if this distance is even, then the distance from a
to b
is odd] For any particular option, half of the remaining options are an even distance and half are an odd distance. Here we arbitrary choose that odd distances correspond to a loss.
compVal = options[compChoice]['value']
userVal = options[userChoice]['value']
numOpns = len(options)
decider = (userVal-compVal) % numOpns
result = 'win'
if decider == 0: result = 'tie'
elif decider%2 == 0: result = 'lose'
# else decider%2 == 1: result = 'win'
print('You '+result+' against the computer!\n')
In this case, hints are definitely a problem if hard coded. So here is a fix for that:
for k,v in options.iteritems():
print(k.upper()+" or "+k+" for "+v['name'])
print('Q or q to quit')
And while we are at it, here is an options dictionary that simulates the correct Rock-Paper-Scissors-Lizard-Spock relationship.
options = {
'r':{'name':'Rock','value':5},
'l':{'name':'Lizard','value':4},
'o':{'name':'Spock','value':3},
's':{'name':'Scissors','value':2},
'p':{'name':'Paper','value':1}
}
-
1\$\begingroup\$ You could probably replace your "magic" 3 with
len(options)
. \$\endgroup\$Dubu– Dubu2017年02月17日 15:14:50 +00:00Commented Feb 17, 2017 at 15:14 -
1\$\begingroup\$ @Dubu The magic number works for 1 option and 3 options but fails for 5 and larger. Changing to
len(options)
won't improve this (and it will make it less readable). But abstracting away the 3 is a good idea. I added an 'advanced check' section which contains a generic solution for any odd number of options. \$\endgroup\$NonlinearFruit– NonlinearFruit2017年02月17日 17:29:27 +00:00Commented Feb 17, 2017 at 17:29 -
\$\begingroup\$ Is
'r':{'name':'Rock','value':1}
a dictionary, within a dictionary?options = { }
is the "parent" dictionary? \$\endgroup\$Richard Peña– Richard Peña2017年02月17日 17:47:05 +00:00Commented Feb 17, 2017 at 17:47 -
\$\begingroup\$ @RichardPeña Yes, it is a dictionary in a dictionary with
options
being the parent. It makes the code a little more readable. \$\endgroup\$NonlinearFruit– NonlinearFruit2017年02月17日 17:53:30 +00:00Commented Feb 17, 2017 at 17:53 -
1\$\begingroup\$ @RichardPeña Thanks for catching that. In Python 3, we need to make the keys into a list:
random.choice(list(options.keys()))
. \$\endgroup\$NonlinearFruit– NonlinearFruit2017年02月17日 21:00:21 +00:00Commented Feb 17, 2017 at 21:00
My second pass,
Improvements:
- Use list and string to find the user's and computer's move easily, instead of doing if-elses.
Never trust user input. Verify that the user input is one of r, p, s or q.
import random user = '' while user != 'q': print('Enter your choice') print('R or r for rock\nP or p for paper\nS or s for scissors\nQ or q to quit') user = input('') user = user.lower() choices = 'rps' if user == 'q': break if user not in choices: print 'invalid choice' continue user = choices.find(user) comp = random.randrange(0, 3) choice = comp choices = ['rock', 'paper', 'scissors'] print('Computer picked:', move_choices[comp]) # compare selection to figure out the winner if user == 0 and comp == 2: print('You win against the computer\n') elif user == 2 and comp == 0: print('You lose against the computer\n') elif user > comp: print('You win against the computer\n') elif user < comp: print('You lose against the computer\n') elif user == comp: print('You tie against the computer\n')
-
1\$\begingroup\$ 'if choices.find(user) == -1': I find it more readable to write 'if user not in choices' \$\endgroup\$MKesper– MKesper2017年02月17日 08:06:26 +00:00Commented Feb 17, 2017 at 8:06
-
\$\begingroup\$ agreed and updated :) \$\endgroup\$Fallen– Fallen2017年02月17日 08:07:29 +00:00Commented Feb 17, 2017 at 8:07
-
1\$\begingroup\$ Your indentation seems to have become corrupted. \$\endgroup\$MKesper– MKesper2017年02月17日 08:09:12 +00:00Commented Feb 17, 2017 at 8:09
Apart from the if/elif improvements, you can also to some other things (here I take tips from all the answers to give you a good solution):
import random
print('Tip:\nR or r for rock\nP or p for paper\nS or s for scissors\nQ or q to quit')
while True:
user = input('Enter you choice\n> ').lower()
options = {'r': 1, 'p': 2, 's': 3}
comp_trans = {1: 'rock', 2: 'paper', 3: 'scissors'}
if user == 'q':
break
elif user not in options.keys():
print('Invalid input, please re try')
continue
comp_choice = random.randint(1, 3)
user_choice = options[user]
print('Computer choosed:', comp_trans[comp_choice])
if (user_choice, comp_choice) in {(1, 3), (2, 1), (3, 2)}:
print('You won!')
elif user_choice == comp_choice:
print('You tied!')
else:
print('You lost!')
Changes:
1) Instead of giving tips every game, we give it once.
2) We dont need to check if user == 'q' each loop (in while statement) because you already do it in the loop
3) if you need only once a value, instead of creating a variable for it and using the variable directly use the value (in displaying the computer choose)
4) we need to check the input, or we may get a KeyError on the line
user_choice = options[user]
5) you asked what does the previous line do: it defines the variable user_choice to the value options[user]. options[user] returns the value in the dictionary options that has the key user. For example, if user = 'r', options[user] will return 1.
-
\$\begingroup\$ Welcome to Code Review, your first answer looks good, enjoy your stay! \$\endgroup\$ferada– ferada2017年02月17日 11:17:16 +00:00Commented Feb 17, 2017 at 11:17
-
\$\begingroup\$ How does
if (user_choice, comp_choice) in {(1, 3), (2, 1), (3, 2)}:
work? If user is: 1 while comp is 3, 2 while comp is 1, 3 while comp is 2? \$\endgroup\$Richard Peña– Richard Peña2017年02月17日 17:42:55 +00:00Commented Feb 17, 2017 at 17:42 -
\$\begingroup\$ @RichardPeña In that case, the other option (
else
) will be chosen, because the other person wins. \$\endgroup\$wizzwizz4– wizzwizz42017年02月18日 09:35:55 +00:00Commented Feb 18, 2017 at 9:35 -
\$\begingroup\$ @RichardPeña you create an object of type tuple (user_choice, comp_choice), and check if there is the same in a set that contains 3 tuples. The difference between set and tuples is that each object in set is unique, and set is not ordered. This increases efficiencity. \$\endgroup\$jthulhu– jthulhu2017年02月18日 12:22:21 +00:00Commented Feb 18, 2017 at 12:22
Python has native lists, tuples and dictionaries (and a number of other data-structures). If you use the builtin data structures wisely you can often drastically simplify your code. In your case you are mapping back and forth between ints (1,2,3), characters ('r', 'p' and 's') and names ('rock',.. etc). Having the ints isn't necessary and the mapping from chars to names is better done with a dict.
import random
options = {"r": "rock", "p": "paper", "s": "scissors"}
while True:
user = input('Choose r for rock, p for paper, s for scissors or q to quit: ')
user = user.lower()
if user == 'q':
break
if user not in options.keys():
continue
choice = random.choice(list(options.keys()))
print('Computer picked:', options[choice])
if choice == user:
print('You tie against the computer\n')
elif (user, choice) in (("r", "s"), ("p", "r"), ("s", "p")):
print('You win against the computer\n')
else:
print('You lose against the computer\n')
Explore related questions
See similar questions with these tags.