3
\$\begingroup\$

I am a beginner in Python and I wanted to see if I could get some feedback on my simple Rock, Paper, Scissors game. I do have previous experience in programming, as I started learning a few years ago but got happy feet and moved through the "basics" for a lot of different languages.

With that said, I plan on sticking with Python for a while until I have programmed a game that I find suitable (I am aiming to start small and end up maybe at a Mario clone).

My biggest issue with this program is that I find it very long-winded, and I am sure there is a way to shorten the code tremendously, I just do not know that solution yet. As well, I do plan on adding something to validate user input but all I know so far are try and except clauses, which I am not sure if they could work.

# a simple game of rock, paper, scissors
import random

opponent = ["Rock", "Paper", "Scissors"]

player1Score = 0

opponentScore = 0

while (True):

    if (player1Score >=3):
        print("Player 1 Wins!!!!!!")
        print("Type 'new game' to play again. (Or type 'exit' to exit.)")
        newGame = input()
        if newGame == "new game":
            player1Score = 0
            opponentScore = 0
            continue
        if newGame == "exit":
            break

    if (opponentScore >=3):
        print("Your opponent Wins!!!!")
        print("Type 'new game' to play again. (Or type 'exit' to exit.)")
        newGame = input()
        if newGame == "new game":
            player1Score = 0
            opponentScore = 0
            continue
        if newGame == "exit":
            break

    print ("Player 1's turn." + " Please type 'Rock', 'Paper', or 'Scissors' to choose your move.")

    opponentMove = random.choice(opponent)
    player1Move = input()

    if player1Move == opponentMove:
        #print (opponentMove) // for debugging
        print("Your score: " + str(int(player1Score)))
        print("Your opponent's score: " + str(int(opponentScore)))
        print ("Tie!" + " Please go again.")
        continue

    if (player1Move == "Rock") and (opponentMove == "Paper"): # rock loses to paper
        opponentScore = opponentScore + 1
        print("Your opponent played: " + str(opponentMove) + ".")
        print("Your score: " + str(int(player1Score)))
        print("Your opponent's score: " + str(int(opponentScore)))
        continue

    if (player1Move == "Paper") and (opponentMove == "Rock"): # paper beats rock
        player1Score = player1Score + 1
        print("Your opponent played: " + str(opponentMove) + ".")
        print("Your score: " + str(int(player1Score)))
        print("Your opponent's score: " + str(int(opponentScore)))
        continue

    if (player1Move == "Paper") and (opponentMove == "Scissors"): # paper loses to scissors
        opponentScore = opponentScore + 1
        print("Your opponent played: " + str(opponentMove) + ".")
        print("Your score: " + str(int(player1Score)))
        print("Your opponent's score: " + str(int(opponentScore)))
        continue

    if (player1Move == "Scissors") and (opponentMove == "Paper"): # scissors beats paper
        player1Score = player1Score + 1
        print("Your opponent played: " + str(opponentMove) + ".")
        print("Your score: " + str(int(player1Score)))
        print("Your opponent's score: " + str(int(opponentScore)))
        continue

    if (player1Move == "Scissors") and (opponentMove == "Rock"): # scissors loses to rock
        opponentScore = opponentScore + 1
        print("Your opponent played: " + str(opponentMove) + ".")
        print("Your score: " + str(int(player1Score)))
        print("Your opponent's score: " + str(int(opponentScore)))
        continue

    if (player1Move == "Rock") and (opponentMove == "Scissors"): # rock beats scissors
        player1Score = player1Score + 1
        print("Your opponent played: " + str(opponentMove) + ".")
        print("Your score: " + str(int(player1Score)))
        print("Your opponent's score: " + str(int(opponentScore)))
        continue
\$\endgroup\$
2
\$\begingroup\$

The code repetition could be cleaned up quickly by replacing your seven if blocks with something like:

print("Your opponent played: " + str(opponentMove) + ".")

if player1Move == opponentMove:
    print ("Tie!" + " Please go again.")
elif (player1Move == "Rock") and (opponentMove == "Paper"): # rock loses to paper
    opponentScore = opponentScore + 1
elif (player1Move == "Paper") and (opponentMove == "Rock"): # paper beats rock
    player1Score = player1Score + 1   
elif (player1Move == "Paper") and (opponentMove == "Scissors"): # paper loses to scissors
    opponentScore = opponentScore + 1
elif (player1Move == "Scissors") and (opponentMove == "Paper"): # scissors beats paper
    player1Score = player1Score + 1
elif (player1Move == "Scissors") and (opponentMove == "Rock"): # scissors loses to rock
    opponentScore = opponentScore + 1
elif (player1Move == "Rock") and (opponentMove == "Scissors"): # rock beats scissors
    player1Score = player1Score + 1
else:
    print ("Invalid choice")

print("Your score: " + str(int(player1Score)))
print("Your opponent's score: " + str(int(opponentScore)))

This eliminates all of the continue statements. Keeping with the same approach you have started, perhaps splitting into cases where player1 wins vs opponent wins is the next improvement:

if player1Move == opponentMove:
    print ("Tie!" + " Please go again.")
elif ((player1Move == "Rock") and (opponentMove == "Paper")) or
     ((player1Move == "Paper") and (opponentMove == "Scissors")) or
     ((player1Move == "Scissors") and (opponentMove == "Paper")):
    player1Score = player1Score + 1
elif ((player1Move == "Paper") and (opponentMove == "Scissors")) or
     ((player1Move == "Scissors") and (opponentMove == "Rock")) or
     ((player1Move == "Rock") and (opponentMove == "Paper")):
    opponentScore = opponentScore + 1
else:
    print ("Invalid choice")
\$\endgroup\$
  • 2
    \$\begingroup\$ This could look even better abstracted into functions that test these values and return True or False, that way you can have elif player_wins(player1Move, opponentMove):. \$\endgroup\$ – SuperBiasedMan May 30 '16 at 9:53
  • 1
    \$\begingroup\$ You have a list. You might want to try using it: try: if player1Move == opponent[opponent.index(opponentMove) - 1]: \$\endgroup\$ – zondo May 30 '16 at 10:00
  • \$\begingroup\$ I really appreciate both of you guys' helpful input, and I am going to rework this program in a few days and repost. I really want to focus on the slimming down aspect of it, so I again really appreciate the feedback! \$\endgroup\$ – CampArawak May 31 '16 at 2:25
1
\$\begingroup\$

First I think you should look at your naming conventions. opponent is where you store a list of the three possible choices. It would make more sense to just name this MOVES or POSSIBLE_MOVES especially as that opens it up to using the values elsewhere.

Input validation is also important. You just trust the player to entre rock paper or scissors, but don't trust your players. Because if they enter "water balloon" then it'll break your game.

You should ensure that they've chosen one of the three possible moves. Put in a function, this is how it might look:

def get_move():

    while True:
        choice = input("Choose your move: ").lower()
        if choice in MOVES:
            return choice
        print("Not a valid move, please enter one of the following:")
        print(", ".join(MOVES))

To break this down, you're using while True to loop forever so that it will keep prompting the user for a value until a valid one is returned from the function (which ends the loop).

You're taking the user's input and using .lower() on it to make it all lowercase, otherwise "ROCK", "Rock" and "rock" will all be separate values (for convenience I would also store MOVES as ["rock", "paper", "scissors"]).

Then you check if the choice is in the MOVES list, and if so it can be returned as it's a valid move for the player. If it's not, the loop continues stating the move was invalid and clarifying what moves are valid.

You can easily call this function now:

player1Move = get_move()
\$\endgroup\$
  • \$\begingroup\$ Thank you for taking the time to review and provide me some feedback. I definitely found out quickly that typing something else would lead to breaking it, especially during the "new game" and "exit" phase. In a few days time I will rework with some other feedback and I will hopefully be able to implement a solid input validation mechanism. Thanks again! \$\endgroup\$ – CampArawak May 31 '16 at 2:27
  • \$\begingroup\$ @CampArawak No problem, glad to help! \$\endgroup\$ – SuperBiasedMan May 31 '16 at 8:55

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.