4
\$\begingroup\$

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(      "=======================================")
    print(      "#######Game############################")

def DisplayOutro():
    print(pad + "\n" + pad + "End of current game")
    print("==============================\n\n\n")


def ClearScreen():
    os.system('cls' if os.name=='nt' else 'clear')


def StartRPSGame():
    bAgain = True

    # Repeat if user wants to play again
    while bAgain:
        # Clear the screen
        ClearScreen()  
        # Display the intro
        DisplayIntro()
        # Get user input
        uInput = GetInput()
        # Handle invalid input
        if uInput < 1:
            print(pad + "Invalid Input")
            DisplayOutro()
            continue
        # Get AI output
        eOutput = GetAIOutput()
        # Display AI output
        print(pad + "Computer: " + str(eOutput))
        # Determine result - win, lose or draw
        result = GetResult(uInput, eOutput)
        # Display results
        print(pad + "Result = " + result + "\n" + pad)
        # Update the scores
        UpdateStats(result)
        # Display the scores
        DisplayStats()
        # Ask if user wants to play again
        print("==============================")
        print(pad + "Play again? (yes=1;no=0)")
        bAgain = bool(input(pad + "Enter: "))
        # Display the outro
        DisplayOutro()
        raw_input("Press enter.")

# Start Rock, Paper Scissors game
StartRPSGame()

Edit: Me and my friend are challenging each other on who will code a better version. Here's his version in c++: Rock, Paper, Scissor Game

\$\endgroup\$
  • 1
    \$\begingroup\$ Since you asked, the Python convention for variable names is lowercase_with_underscores, which the exception of classes, which are CamelCase. The Python style guide is PEP 8. \$\endgroup\$ – alexwlchan Mar 1 '16 at 7:30
  • \$\begingroup\$ Love the modulo use. That is the smartest way of solving rock-paper-scissors \$\endgroup\$ – Arnab Datta Mar 8 '16 at 10:18
  • \$\begingroup\$ @Arnab Datta Yeah, I know. This can be easily modified to support more weapons (3+ weapons). \$\endgroup\$ – PotCoder Mar 10 '16 at 9:01
  • \$\begingroup\$ @PotCoder github.com/arnabkd/roshambo-py/blob/master/roshambo-simple.py \$\endgroup\$ – Arnab Datta Mar 10 '16 at 10:14
3
\$\begingroup\$

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).

\$\endgroup\$
  • \$\begingroup\$ Naming conventions: I have no idea of the naming conventions that are followed in python. I'll go through them and update my code following the naming conventions. Usage of docstrings: I've learned about these but never really used them. Seems like they really help to understand what the code does. I'll be using them from now on to make the reader understand better. But one question, is it necessary to add a docstring to every function? \$\endgroup\$ – PotCoder Mar 2 '16 at 15:47
  • \$\begingroup\$ Code too split up: I agree that the code seems to be to split up. GetResult and UpdateStats could be in a single function. But what if I wanted to do something between those two? Like, who knows. StartRPSGame overcommented: I once again agree with this. My friend too told me that I was using comments too often even though the code speaks for itself. But in case the code is not very easy to understand by looking merely and requires the algorithm to be explained in a pseudocode, should I use comments or docstrings? 1LastThing. How do I add new lines in comments? <br/> tag aint working \$\endgroup\$ – PotCoder Mar 2 '16 at 15:58
  • \$\begingroup\$ @PotCoder I wouldn't add one to every function as some can be pretty easy to follow. But I personally do have one most of the time (and very few other comments). If you might need to split them up, I'd at least make a single end game function that calls both and passes values between them. You've overcomplicated it right now with how they're split. Functions should make code neater, not more complicated to use. Lastly you can't format comments very much, they only have mini-Markdown formatting. \$\endgroup\$ – SuperBiasedMan Mar 3 '16 at 17:21

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.