Some your variable naming could be clearer. 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 @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).
Some your variable naming could be clearer. 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).
Some your variable naming could be clearer. 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).
Some your variable naming could be clearer. 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).