patternpythonMinor
Simple Rock Paper Scissors Game
Viewed 0 times
scissorspapersimplegamerock
Problem
Note: This code was first posted on another forum by me under the name Candy. Read this note before you declare this code copied.
Since I was bored, I decided to code a Rock Paper Scissors game in Python in my iOS device (There are offline interpreters available). The very first version of my code was ugly and large, so I decided to re-write the whole thing from scratch. I have tried to keep the code short because coding in mobile was a bit difficult.
I would like to know your suggestions for improvement of this game. Also point out any bad/wrong practices that you find. I don't even know whether I follow the right way of naming variables. So please tell me the 'correct' naming conventions in Python. And yeah, try not to be too harsh on this one. After all, it's my first game in Python :)
```
import random
import os
pad = "|| "
def GetInput():
print(pad + "(1)Rock, (2)Paper or (3)Scissor?")
sInput = raw_input(pad + "User: ").lower()
if sInput in ["rock", "r", "1"]:
return 1
elif sInput in ["paper", "l", "2"]:
return 2
elif sInput in ["scissor", "s", "3"]:
return 3
else:
return -1
def GetResult(pA, pB):
res = (3 + pA - pB) % 3
if res == 1:
return "win"
elif res == 2:
return "lose"
elif res == 0:
return "draw"
pass
def GetAIOutput():
out = random.randint(1, 3)
return out
Stats = { "Win" : 0, "Lose" : 0, "Draw" : 0, "Total" : 0 }
def UpdateStats(state):
Stats["Total"] += 1
if state == "win":
Stats["Win"] += 1
elif state == "lose":
Stats["Lose"] += 1
else:
Stats["Draw"] += 1
def DisplayStats():
print("///////Stats//////////////////////////")
for key, value in Stats.items():
print(pad + key + " : " + str(value))
print(pad)
def DisplayIntro():
print( "=======================================")
print(pad + " Rock, Paper, Scissors")
print(pad + " by CandyV3rm")
print
Since I was bored, I decided to code a Rock Paper Scissors game in Python in my iOS device (There are offline interpreters available). The very first version of my code was ugly and large, so I decided to re-write the whole thing from scratch. I have tried to keep the code short because coding in mobile was a bit difficult.
I would like to know your suggestions for improvement of this game. Also point out any bad/wrong practices that you find. I don't even know whether I follow the right way of naming variables. So please tell me the 'correct' naming conventions in Python. And yeah, try not to be too harsh on this one. After all, it's my first game in Python :)
```
import random
import os
pad = "|| "
def GetInput():
print(pad + "(1)Rock, (2)Paper or (3)Scissor?")
sInput = raw_input(pad + "User: ").lower()
if sInput in ["rock", "r", "1"]:
return 1
elif sInput in ["paper", "l", "2"]:
return 2
elif sInput in ["scissor", "s", "3"]:
return 3
else:
return -1
def GetResult(pA, pB):
res = (3 + pA - pB) % 3
if res == 1:
return "win"
elif res == 2:
return "lose"
elif res == 0:
return "draw"
pass
def GetAIOutput():
out = random.randint(1, 3)
return out
Stats = { "Win" : 0, "Lose" : 0, "Draw" : 0, "Total" : 0 }
def UpdateStats(state):
Stats["Total"] += 1
if state == "win":
Stats["Win"] += 1
elif state == "lose":
Stats["Lose"] += 1
else:
Stats["Draw"] += 1
def DisplayStats():
print("///////Stats//////////////////////////")
for key, value in Stats.items():
print(pad + key + " : " + str(value))
print(pad)
def DisplayIntro():
print( "=======================================")
print(pad + " Rock, Paper, Scissors")
print(pad + " by CandyV3rm")
Solution
Some your variable naming could be clearer.
You could also improve clarity by adding docstrings that explain what each of your functions do.
A docstring simply explains what a function does, how it works, how you should use it. They've very helpful for understanding functions. For an even more useful one, consider your clear screen function. As clear as the name is, it doesn't fully explain it's need:
def clear_screen():
"""Calls the relevant clear screen command from the user's OS."""
In general, I think you have your functions too split up. Is there any need to have
Lastly your commenting in the
All of those are perfectly clear without comments. Your concise names tell me the basics about the function, so why double it up with a comment saying practically the same thing? (using mostly the same words too).
pad seems to be for padding, but it's a bit unclear that you're using it to format printing calls. Aside from a different name (padding at least) you could improve this by using the naming convention of constants. Python's naming convention for constants is to use UPPER_SNAKE_CASE, ie. PADDING. It signals that this is a constant value that you wont be changing. As @alexwlchan noted, non constants (including functions) are lower_snake_case and classes are in PascalCase. Aside from that, sInput, eOutput, pA, pB. Using words is more verbose but it's going to communicate much more than short codes. Even if they seem clear to you, you can't be sure people will correctly interpret what word they're stand ins for.You could also improve clarity by adding docstrings that explain what each of your functions do.
GetAIOutput is a short simple function, but explaining it's usage is still good to remove ambiguity.def ai_choice():
"""Returns a random int from 1 - 3 as the AI's choice."""
return random.randint(1, 3)A docstring simply explains what a function does, how it works, how you should use it. They've very helpful for understanding functions. For an even more useful one, consider your clear screen function. As clear as the name is, it doesn't fully explain it's need:
def clear_screen():
"""Calls the relevant clear screen command from the user's OS."""
os.system('cls' if os.name=='nt' else 'clear')In general, I think you have your functions too split up. Is there any need to have
GetResult and UpdateStats as separate functions? Wouldn't it be easier to get the result, update stats from that and then print the result message all in one function. Generally functions should have one 'job', but that's different to performing just a single operation. The two above functions make sense as single functions. But sometimes multiple related operations are worth combining into one function. Specifically the game's results, particularly if there's no reason you'd want to call the component parts separately.Lastly your commenting in the
StartRPSGame doesn't give your good function names enough credit# Clear the screen
ClearScreen()
# Display the intro
DisplayIntro()
# Get user input
uInput = GetInput()All of those are perfectly clear without comments. Your concise names tell me the basics about the function, so why double it up with a comment saying practically the same thing? (using mostly the same words too).
Code Snippets
def ai_choice():
"""Returns a random int from 1 - 3 as the AI's choice."""
return random.randint(1, 3)os.system('cls' if os.name=='nt' else 'clear')# Clear the screen
ClearScreen()
# Display the intro
DisplayIntro()
# Get user input
uInput = GetInput()Context
StackExchange Code Review Q#121524, answer score: 3
Revisions (0)
No revisions yet.