Skip to main content
Code Review

Return to Answer

replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/
Source Link

Like @wb9688 @wb9688 mentioned in the comments, the way you currently determine the winner is fine and working, but it does not scale very well. If you wanted to add two more possible moves (for example lizard and spock), your if..else tree will become very large and hard to maintain.

It would therefore be better to just save which move beats which other move (for example in a dictionary):

beats = {'paper': 'scissors',
 'rock': 'paper',
 'scissors': 'rock'}

Then you can just do:

computer_choice = getComp()
user_choice = getUser()
if user_choice == beats[computer_choice]:
 print "{} beats {}, you win!".format(user_choice, computer_choice)
elif computer_choice == beats[user_choice]:
 print "{} beats {}, computer wins!".format(computer_choice, user_choice)
else:
 print "Tie."

If you also want to add the verbs, just add a dictionary for that as well:

verb = {'rock': 'smashes',
 'paper': 'smothers',
 'scissors': 'slices'}

And then do:

if user_choice == beats[computer_choice]:
 print "{} {} {}, you win!".format(user_choice, verb[user_choice], computer_choice)
...

You can have a look at my answer to a similar question on how easy it is to extend this framework to more moves: Stone, Paper, Scissors in Python Stone, Paper, Scissors in Python

Note that you would have to change the structure of verb to a nested dictionary, because "Paper disproves Spock" instead of "Paper smothers Spock".

Your function getUser should be modified, as well. Right now, you call it recursively in case the user enters an illegal move. However, you never return the result of that recursive call!

In python, using a while loop is usually preferred in this case, because Python does not do tail recursion and so will hit a maximum recursion depth at some point:

def getUser():
 while True:
 user_input = raw_input("Make a choice: {} ".format(beats.keys()))
 if user_input in beats:
 return user_input
 print "Invalid choice."

Note: In Python-3.x, use input instead of raw_input and print(x) instead of print x.

Finally, in your function getComp, you can just return the result right away. However, it needs to be modified to use the keys of beats, instead of integers:

import random
def getComp():
 return random.choice(beats.keys())

Like @wb9688 mentioned in the comments, the way you currently determine the winner is fine and working, but it does not scale very well. If you wanted to add two more possible moves (for example lizard and spock), your if..else tree will become very large and hard to maintain.

It would therefore be better to just save which move beats which other move (for example in a dictionary):

beats = {'paper': 'scissors',
 'rock': 'paper',
 'scissors': 'rock'}

Then you can just do:

computer_choice = getComp()
user_choice = getUser()
if user_choice == beats[computer_choice]:
 print "{} beats {}, you win!".format(user_choice, computer_choice)
elif computer_choice == beats[user_choice]:
 print "{} beats {}, computer wins!".format(computer_choice, user_choice)
else:
 print "Tie."

If you also want to add the verbs, just add a dictionary for that as well:

verb = {'rock': 'smashes',
 'paper': 'smothers',
 'scissors': 'slices'}

And then do:

if user_choice == beats[computer_choice]:
 print "{} {} {}, you win!".format(user_choice, verb[user_choice], computer_choice)
...

You can have a look at my answer to a similar question on how easy it is to extend this framework to more moves: Stone, Paper, Scissors in Python

Note that you would have to change the structure of verb to a nested dictionary, because "Paper disproves Spock" instead of "Paper smothers Spock".

Your function getUser should be modified, as well. Right now, you call it recursively in case the user enters an illegal move. However, you never return the result of that recursive call!

In python, using a while loop is usually preferred in this case, because Python does not do tail recursion and so will hit a maximum recursion depth at some point:

def getUser():
 while True:
 user_input = raw_input("Make a choice: {} ".format(beats.keys()))
 if user_input in beats:
 return user_input
 print "Invalid choice."

Note: In Python-3.x, use input instead of raw_input and print(x) instead of print x.

Finally, in your function getComp, you can just return the result right away. However, it needs to be modified to use the keys of beats, instead of integers:

import random
def getComp():
 return random.choice(beats.keys())

Like @wb9688 mentioned in the comments, the way you currently determine the winner is fine and working, but it does not scale very well. If you wanted to add two more possible moves (for example lizard and spock), your if..else tree will become very large and hard to maintain.

It would therefore be better to just save which move beats which other move (for example in a dictionary):

beats = {'paper': 'scissors',
 'rock': 'paper',
 'scissors': 'rock'}

Then you can just do:

computer_choice = getComp()
user_choice = getUser()
if user_choice == beats[computer_choice]:
 print "{} beats {}, you win!".format(user_choice, computer_choice)
elif computer_choice == beats[user_choice]:
 print "{} beats {}, computer wins!".format(computer_choice, user_choice)
else:
 print "Tie."

If you also want to add the verbs, just add a dictionary for that as well:

verb = {'rock': 'smashes',
 'paper': 'smothers',
 'scissors': 'slices'}

And then do:

if user_choice == beats[computer_choice]:
 print "{} {} {}, you win!".format(user_choice, verb[user_choice], computer_choice)
...

You can have a look at my answer to a similar question on how easy it is to extend this framework to more moves: Stone, Paper, Scissors in Python

Note that you would have to change the structure of verb to a nested dictionary, because "Paper disproves Spock" instead of "Paper smothers Spock".

Your function getUser should be modified, as well. Right now, you call it recursively in case the user enters an illegal move. However, you never return the result of that recursive call!

In python, using a while loop is usually preferred in this case, because Python does not do tail recursion and so will hit a maximum recursion depth at some point:

def getUser():
 while True:
 user_input = raw_input("Make a choice: {} ".format(beats.keys()))
 if user_input in beats:
 return user_input
 print "Invalid choice."

Note: In Python-3.x, use input instead of raw_input and print(x) instead of print x.

Finally, in your function getComp, you can just return the result right away. However, it needs to be modified to use the keys of beats, instead of integers:

import random
def getComp():
 return random.choice(beats.keys())
added 35 characters in body
Source Link
Graipher
  • 41.7k
  • 7
  • 70
  • 134

Like @wb9688 mentioned in the comments, the way you currently determine the winner is fine and working, but it does not scale very well. If you wanted to add two more possible moves (for example lizard and spock), your if..else tree will become very large and hard to maintain.

It would therefore be better to just save which move beats which other move (for example in a dictionary):

beats = {'paper': 'scissors',
 'rock': 'paper',
 'scissors': 'rock'}

Then you can just do:

computer_choice = getComp()
user_choice = getUser()
if user_choice == beats[computer_choice]:
 print "{} beats {}, you win!".format(user_choice, computer_choice)
elif computer_choice == beats[user_choice]:
 print "{} beats {}, computer wins!".format(computer_choice, user_choice)
else:
 print "Tie."

If you also want to add the verbs, just add a dictionary for that as well:

verb = {'rock': 'smashes',
 'paper': 'smothers',
 'scissors': 'slices'}

And then do:

if user_choice == beats[computer_choice]:
 print "{} {} {}, you win!".format(user_choice, verb[user_choice], computer_choice)
...

You can have a look at my answer to a similar question on how easy it is to extend this framework to more moves: Stone, Paper, Scissors in Python

Note that you would have to change the structure of verb to a nested dictionary, because "Paper disproves Spock" instead of "Paper smothers Spock".

Your function getUser should be modified, as well. Right now, you call it recursively in case the user enters an illegal move. However, you never return the result of that recursive call!

In python, using a while loop is usually preferred in this case, because Python does not do tail recursion and so will hit a maximum recursion depth at some point:

def getUser():
 while True:
 user_input = raw_input("Make a choice: {} ".format(beats.keys()))
 if user_input in beats:
 return user_input
 print "Invalid choice."

Note: In Python-3.x, use input instead of raw_input and print(x) instead of print x.

Finally, in your function getComp, you can just return the result right away. However, it needs to be modified to use the keys of beats, instead of integers:

import random
def getComp():
 return random.choice(beats.keys())

Like @wb9688 mentioned in the comments, the way you currently determine the winner is fine and working, but it does not scale very well. If you wanted to add two more possible moves (for example lizard and spock), your if..else tree will become very large and hard to maintain.

It would therefore be better to just save which move beats which other move (for example in a dictionary):

beats = {'paper': 'scissors',
 'rock': 'paper',
 'scissors': 'rock'}

Then you can just do:

computer_choice = getComp()
user_choice = getUser()
if user_choice == beats[computer_choice]:
 print "{} beats {}, you win!".format(user_choice, computer_choice)
elif computer_choice == beats[user_choice]:
 print "{} beats {}, computer wins!".format(computer_choice, user_choice)
else:
 print "Tie."

If you also want to add the verbs, just add a dictionary for that as well:

verb = {'rock': 'smashes',
 'paper': 'smothers',
 'scissors': 'slices'}

And then do:

if user_choice == beats[computer_choice]:
 print "{} {} {}, you win!".format(user_choice, verb[user_choice], computer_choice)
...

You can have a look at my answer to a similar question on how easy it is to extend this framework to more moves: Stone, Paper, Scissors in Python

Note that you would have to change the structure of verb to a nested dictionary, because "Paper disproves Spock" instead of "Paper smothers Spock".

Your function getUser should be modified, as well. Right now, you call it recursively in case the user enters an illegal move. However, you never return the result of that recursive call!

In python, using a while loop is usually preferred in this case, because Python does not do tail recursion and so will hit a maximum recursion depth at some point:

def getUser():
 while True:
 user_input = raw_input("Make a choice: {} ".format(beats.keys()))
 if user_input in beats:
 return user_input
 print "Invalid choice."

Note: In Python-3.x, use input instead of raw_input.

Finally, in your function getComp, you can just return the result right away. However, it needs to be modified to use the keys of beats, instead of integers:

import random
def getComp():
 return random.choice(beats.keys())

Like @wb9688 mentioned in the comments, the way you currently determine the winner is fine and working, but it does not scale very well. If you wanted to add two more possible moves (for example lizard and spock), your if..else tree will become very large and hard to maintain.

It would therefore be better to just save which move beats which other move (for example in a dictionary):

beats = {'paper': 'scissors',
 'rock': 'paper',
 'scissors': 'rock'}

Then you can just do:

computer_choice = getComp()
user_choice = getUser()
if user_choice == beats[computer_choice]:
 print "{} beats {}, you win!".format(user_choice, computer_choice)
elif computer_choice == beats[user_choice]:
 print "{} beats {}, computer wins!".format(computer_choice, user_choice)
else:
 print "Tie."

If you also want to add the verbs, just add a dictionary for that as well:

verb = {'rock': 'smashes',
 'paper': 'smothers',
 'scissors': 'slices'}

And then do:

if user_choice == beats[computer_choice]:
 print "{} {} {}, you win!".format(user_choice, verb[user_choice], computer_choice)
...

You can have a look at my answer to a similar question on how easy it is to extend this framework to more moves: Stone, Paper, Scissors in Python

Note that you would have to change the structure of verb to a nested dictionary, because "Paper disproves Spock" instead of "Paper smothers Spock".

Your function getUser should be modified, as well. Right now, you call it recursively in case the user enters an illegal move. However, you never return the result of that recursive call!

In python, using a while loop is usually preferred in this case, because Python does not do tail recursion and so will hit a maximum recursion depth at some point:

def getUser():
 while True:
 user_input = raw_input("Make a choice: {} ".format(beats.keys()))
 if user_input in beats:
 return user_input
 print "Invalid choice."

Note: In Python-3.x, use input instead of raw_input and print(x) instead of print x.

Finally, in your function getComp, you can just return the result right away. However, it needs to be modified to use the keys of beats, instead of integers:

import random
def getComp():
 return random.choice(beats.keys())
added 137 characters in body
Source Link
Graipher
  • 41.7k
  • 7
  • 70
  • 134

Like @wb9688@wb9688 mentioned in the comments, the way you currently determine the winner is fine and working, but it does not scale very well. If you wanted to add two more possible moves (for example lizard and spock), your if..else tree will become very large and hard to maintain.

It would therefore be better to just save which move beats which outherother move (for example in a dictionary).

An example would be:

beats = {'paper': 'scissors',
 'rock': 'paper',
 'scissors': 'rock'}

Then you can just do:

computer_choice = getComp()
user_choice = getUser()
if user_choice == beats[computer_choice]:
 print "{} beats {}, you win!".format(user_choice, computer_choice)
elif computer_choice == beats[user_choice]:
 print "{} beats {}, computer wins!".format(computer_choice, user_choice)
else:
 print "Tie."

If you also want to add the verbs, just add a dictionary for that as well:

verb = {'rock': 'smashes',
 'paper': 'smothers',
 'scissors': 'slices'}

And then do:

if user_choice == beats[computer_choice]:
 print "{} {} {}, you win!".format(user_choice, verb[user_choice], computer_choice)
...

You can have a look at my answer to a similar question on how easy it is to extend this framework to more moves: Stone, Paper, Scissors in Python

Note that you would have to change the structure of verb to a nested dictionary, because "Paper disproves Spock" instead of "Paper smothers Spock".

Your function getUser should be modified, as well. Right now, you call it recursively in case the user enters an illegal move. However, you never return the result of that recursive call!

In python, using a while loop is usually preferred in this case, because Python does not do tail recursion and so will hit a maximum recursion depth at some point:

def getUser():
 while True:
 user_input = raw_input("Make a choice: {} ".format(beats.keys()))
 if user_input in beats:
 return user_input
 print "Invalid choice."

Note: In Python-3.x, use input instead of raw_input.

Finally, in your function getComp, you can just return the result right away. However, it needs to be modified to use the keys of beats, instead of integers:

import random
def getComp():
 return random.choice(beats.keys())

Like @wb9688 mentioned in the comments, the way you currently determine the winner is fine and working, but it does not scale very well. If you wanted to add two more possible moves (for example lizard and spock), your if..else tree will become very large and hard to maintain.

It would therefore be better to just save which move beats which outher move (for example in a dictionary).

An example would be

beats = {'paper': 'scissors',
 'rock': 'paper',
 'scissors': 'rock'}

Then you can just do:

computer_choice = getComp()
user_choice = getUser()
if user_choice == beats[computer_choice]:
 print "{} beats {}, you win!".format(user_choice, computer_choice)
elif computer_choice == beats[user_choice]:
 print "{} beats {}, computer wins!".format(computer_choice, user_choice)
else:
 print "Tie."

If you also want to add the verbs, just add a dictionary for that as well:

verb = {'rock': 'smashes',
 'paper': 'smothers',
 'scissors': 'slices'}

And then do:

if user_choice == beats[computer_choice]:
 print "{} {} {}, you win!".format(user_choice, verb[user_choice], computer_choice)
...

You can have a look at my answer to a similar question on how easy it is to extend this framework to more moves: Stone, Paper, Scissors in Python

Your function getUser should be modified. Right now, you call it recursively in case the user enters an illegal move. However, you never return the result of that recursive call!

In python, using a while loop is usually preferred in this case, because Python does not do tail recursion and so will hit a maximum recursion depth at some point:

def getUser():
 while True:
 user_input = raw_input("Make a choice: {} ".format(beats.keys()))
 if user_input in beats:
 return user_input
 print "Invalid choice."

Like @wb9688 mentioned in the comments, the way you currently determine the winner is fine and working, but it does not scale very well. If you wanted to add two more possible moves (for example lizard and spock), your if..else tree will become very large and hard to maintain.

It would therefore be better to just save which move beats which other move (for example in a dictionary):

beats = {'paper': 'scissors',
 'rock': 'paper',
 'scissors': 'rock'}

Then you can just do:

computer_choice = getComp()
user_choice = getUser()
if user_choice == beats[computer_choice]:
 print "{} beats {}, you win!".format(user_choice, computer_choice)
elif computer_choice == beats[user_choice]:
 print "{} beats {}, computer wins!".format(computer_choice, user_choice)
else:
 print "Tie."

If you also want to add the verbs, just add a dictionary for that as well:

verb = {'rock': 'smashes',
 'paper': 'smothers',
 'scissors': 'slices'}

And then do:

if user_choice == beats[computer_choice]:
 print "{} {} {}, you win!".format(user_choice, verb[user_choice], computer_choice)
...

You can have a look at my answer to a similar question on how easy it is to extend this framework to more moves: Stone, Paper, Scissors in Python

Note that you would have to change the structure of verb to a nested dictionary, because "Paper disproves Spock" instead of "Paper smothers Spock".

Your function getUser should be modified, as well. Right now, you call it recursively in case the user enters an illegal move. However, you never return the result of that recursive call!

In python, using a while loop is usually preferred in this case, because Python does not do tail recursion and so will hit a maximum recursion depth at some point:

def getUser():
 while True:
 user_input = raw_input("Make a choice: {} ".format(beats.keys()))
 if user_input in beats:
 return user_input
 print "Invalid choice."

Note: In Python-3.x, use input instead of raw_input.

Finally, in your function getComp, you can just return the result right away. However, it needs to be modified to use the keys of beats, instead of integers:

import random
def getComp():
 return random.choice(beats.keys())
Source Link
Graipher
  • 41.7k
  • 7
  • 70
  • 134
Loading
lang-py

AltStyle によって変換されたページ (->オリジナル) /