4
\$\begingroup\$

This is an assignment that I have already turned in. It works great, but I was wondering if there are any flaws in the code, or how to write it in a more pythonic way.

import random
tie = 0
pcWon = 0
playerWon = 0

# Displays program information, starts main play loop, after main loop is executed (user pressing 4 in menu), display
# the scores to the user.
def main():
    print("A game or rock, paper, or scissors!")
    playGame = True
    while playGame:
        playGame = play()
    displayScoreBoard()
    prompt = input("press enter to exit")


# displays the menu for user, if input ==4, playGame in the calling function (main()) is False, terminating the program.
# Generate a random int 1-3, evaluate the user input with the computer input, update globals accordingly, returning True
# to playGame, resulting in the loop in the calling function (main()) to continue.
def play():
    playerChoice = int(playerMenu())
    if playerChoice == 4:
        return 0
    else:
        pcChoice = pcGenerate()
        outcome = evaluateGame(playerChoice, pcChoice)
        updateScoreBoard(outcome)
        return 1


# prints the menu, the player selects a menu item, the input is validated, if the input is valid, returned the input, if
# the input is not valid, continue to prompt for a valid input
# 1 - rock
# 2 - paper
# 3 - scissors
# 4 - quit
def playerMenu():
    print("Select a choice: \n [1]: Rock \n [2]: Paper \n [3]: Scissors \n [4]: Quit")
    menuSelect = input("Enter menu selection: ")
    while not validateInput(menuSelect):
        invalidChoice(menuSelect)
        menuSelect = input("Enter a correct value: ")
    return menuSelect


# if the user doesn't input a 1-4 then return false, resulting in prompting the user for another value. If the value
# is valid, return True
# takes 1 argument
# menuSelection - value user entered prior
def validateInput(menuSelection):
    if menuSelection == "1" or menuSelection == "2" or menuSelection == "3" or menuSelection == "4":
        return True
    else:
        return False


# return a random integer 1-3 to determine pc selection
# 1 - rock
# 2 - paper
# 3 - scissors
def pcGenerate():
    pcChoice = random.randint(1,3)
    return pcChoice


# evaluate if the winner is pc or player or tie, return value accordingly
# 0 - tie
# 1 - player won
# -1 - pc won
def evaluateGame(playerChoice, pcChoice):
    if playerChoice == 1:
        print("You have chosen rock.")
        if pcChoice == 1:
            #tie
            print("Computer has chose rock as well. TIE!")
            return 0
        elif pcChoice == 2:
            #paper covers rock - pc won
            print("The computer has chosen paper. Paper covers rock. You LOSE!")
            return -1
        else:
            #rock breaks scissors - player won
            print("The computer has chosen scissors. Rock breaks scissors. You WIN!")
            return 1
    elif playerChoice == 2:
        print("You have chosen paper.")
        if pcChoice == 1:
            #paper covers rock - player won
            print("The computer has chosen rock. Paper covers rock. You WIN!")
            return 1
        elif pcChoice == 2:
            #tie
            print("The computer has chosen paper as well. TIE!")
            return 0
        else:
            #scissors cut paper - pc won
            print("The computer has chosen scissors. Scissors cut paper. You LOSE!")
            return -1
    else: #plyer choice defaults to 3
        print("You have chosen scissors")
        if pcChoice == 1:
            #rock breaks scissors - pc won
            print("The computer has chosen rock. Rock breaks scissors. You LOSE!")
            return -1
        elif pcChoice == 2:
            #scissors cut paper - player won
            print("The computer has chosen paper. Scissors cut paper. You WIN!")
            return 1
        else: #pc defaults to scissors
            #tie
            print("The computer has chosen scissors as well. TIE!")
            return 0


# Update track of ties, player wins, and computer wins
def updateScoreBoard(gameStatus):
    global tie, playerWon, pcWon
    if gameStatus == 0:
        tie +=1
    elif gameStatus == 1:
        playerWon += 1
    else:
        pcWon += 1


# If user input is invalid, let them know.
def invalidChoice(menuSelect):
    print(menuSelect, "is not a valid option. Please use 1-4")


# Print the scores before terminating the program.
def displayScoreBoard():
    global tie, playerWon, pcWon
    print("Extra Credit:\nTies:", tie, "\tPlayer Wins:", playerWon, "\tComputer Wins:", pcWon)

main()
\$\endgroup\$
4
\$\begingroup\$
def playerMenu():
    print('Select a choice: \n\n\t [1]: Rock \n\t [2]: Paper \n\t [3]: Scissors \n\t [4]: Quit\n')
    menuSelect = input('Enter menu selection: ')
    while menuSelect not in ['1','2','3','4']:
        print(menuSelect, 'is not a valid option. Please use 1-4')
        menuSelect = input('Enter a correct value: ')
    return menuSelect

Note I got rid of two unnecessary functions, validateInput and invalidChoice. Doing so increases performance and readability. However, don't play code golf. Separating sub-tasks and having uniform levels of abstraction is important. But, these were one line a piece.

The in keyword makes things simple, faster, and easier to read.

I will say also, you could use a dictionary to match the menuSelect to the corresponding option, ie. rock, paper, or scissors. I'd set that up in player menu upon their selection..

options = {1: 'rock', 2: 'paper', 3: 'scissors'}

Then you can get rid of all those if statements. It should be a warning sign whenever there are too many if statements.

You only need to test if the player's option equals the computer's option once. But you test it three times.

You could use code like:

if playerChoice == pcChoice:
    print("It's a tie! The computer has chosen", pcChoice, "as well!")
elif playerChoice == rock and pcChoice == scissors or
        playerChoice == paper and pcChoice == rock:
    print('You win! The computer has chosen', pcChoice + '.', playerChoice, 'beats', pcChoice +'!')
else:
    print('You lose.. The computer has chosen', pcChoice + '.', pcChoice, 'beats', playerChoice +'!')

There's probably some more, but I ran out of time

\$\endgroup\$
2
\$\begingroup\$
def validateInput(menuSelection):
if menuSelection == "1" or menuSelection == "2" or menuSelection == "3" or menuSelection == "4":
    return True
else:
    return False

You could use dictionaries for things like this, e.g.

def validateInput(menuSelection):
    return {1: True, 2: True, 3: True, 4: True}.get(menuSelection, False)

One problem with this is that you create the dict on every function call. To alleviate this:

inputOpts = {1: True, 2: True, 3: True, 4: True}
isValid = inputOpts.get(menuSelection, False)
\$\endgroup\$
1
\$\begingroup\$

You can simplify your validateInput function as follows ...

def validateInput(menuSelection):
    return menuSelection in ["1", "2", "3", "4"]

if menuSelection matches one of the elements in the list, it returns True. Otherwise, it returns False.

\$\endgroup\$
  • 2
    \$\begingroup\$ Could you add a bit explanation to your answer ? I really feel like a code only answer is not ideal as a review, as it don't really help the OP. \$\endgroup\$ – Marc-Andre Feb 28 '14 at 15:43

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.