8
\$\begingroup\$

I am a newbie to Python and am trying to do the Rock, Paper and Scissors game to revise my knowledge that I have earned so far.

Overview of the program:

  • This is a two-player game
  • Ask for player plays (including name, their choice: rock or paper or scissors using input)
  • Print out a message of winner
  • Ask if the user want to start a new game

This code is written in procedural style.

Can anyone show me some weaknesses in my code and how I can improve on them?

def beats(a, b): 
    tup = ('rock', 'paper', 'scissors')
    if (a == tup[1] and b == tup[2]) or (a == tup[2] and b == tup[1]):
        return tup[2]
    elif (a == tup[1] and b == tup[0]) or (a == tup[0] and b == tup[1]):
        return tup[1]
    elif (a == tup[0] and b == tup[2]) or (a == tup[2] and b == tup[0]):
        return tup[0]


def players_input():
    name = input('What is your name?: ')
    while True:
        decision = input('Paper, Rock, or Scissors?: ')
        if decision == 'rock' or decision == 'paper' or decision == 'scissors':
            break
        else:
            print("Invalid choice! Try again!")
            continue
    return '{} chooses {}'.format(name, decision)


def play():
    player1 = players_input()
    player2 = players_input()
    print('{}\n{}'.format(player1, player2))
    name1, verb, decision1 = player1.split(" ")
    name2, verb, decision2 = player2.split(" ")
    returnvalue = beats(decision1.lower(), decision2.lower())
    if decision1.lower() == returnvalue and decision2.lower() != returnvalue:
        print('{} wins'.format(name1))
    elif decision2.lower() == returnvalue and decision1.lower() != returnvalue:
        print('{} wins'.format(name2))
    else:
        print('It\'s a tie')


def play_again():
    while True:
        choice = input('Do you want to continue?(Y/N): ')
        if choice.upper() == "Y":
            play()
        elif choice.upper() == "N":
            print("Game over")
            break
        else:
            print("Invalid Input! Try again!")


play()
play_again()
\$\endgroup\$
4
\$\begingroup\$

I recommend simplifying:

if decision == 'rock' or decision == 'paper' or decision == 'scissors':

to:

if decision.lower() in ('rock', 'paper', 'scissors'):

This will check whether the decision exists in a given tuple.

This helps simplifying the idea "If a variable contains one of these values".

Another example is "If variable x contains either 1, 3, or 5." Instead of coding it like this:

if x == 5 or x == 3 or x == 1

It can be simplified into:

if x in (1, 3, 5)

See this Gist to see my suggestion as a whole.

\$\endgroup\$
  • \$\begingroup\$ I do not catch the idea of if x == 5 or x == 3 or x == 1? What is x, 1, 3 and 5? Can you help me explain it? \$\endgroup\$ – Le Quynh Anh Jul 4 '17 at 18:14
  • \$\begingroup\$ I'm just using it as an example. It's another way of saying "If a variable holds one of these values." \$\endgroup\$ – Sometowngeek Jul 4 '17 at 18:21
3
\$\begingroup\$

First of all, it is good you are reviewing your knowledge.
Secondly, I would like to add to Sometowngeek's suggestion. Implement his one.

Thirdly, instead of writing:

name1, verb, decision1 = player1.split(" ")

you could write:

name1, decision1 = player1.split(" chooses ")

I made some changes to your play() function:

def play():
    player1 = players_input()
    player2 = players_input()

    print('{}\n{}'.format(player1, player2))

    name1, decision1 = player1.split(" chooses ")
    name2, decision2 = player2.split(" chooses ")

    if decision1 == decision2:
        print('It\'s a tie')
    else:
        returnValue = beats(decision1, name1, decision2, name2)
        print('{} wins'.format(returnValue))

And some more changes to your beats() function:

def beats(a, name1, b, name2):
    tup = ('rock', 'paper', 'scissors')
    if (a == tup[0] and b == tup[2]) or (a == tup[1] and b == tup[0]) or (a == tup[2] and b == tup[1]):
        return name1
    else:
        return name2

In beats() function, it is better to return the names of the players; so, you will not use if in your play() function.

Finally, good luck with your future coding!

\$\endgroup\$
  • \$\begingroup\$ I had thought that the split function can only be passed in a delimeter (like space, -, so on). Thank you, I have realized lots of things. \$\endgroup\$ – Le Quynh Anh Jul 6 '17 at 8:19
  • \$\begingroup\$ @BlahBlah, In the beats() method, there's a possibility of tie. Might want to add something to handle that. :) \$\endgroup\$ – Sometowngeek Jul 6 '17 at 20:45
  • 2
    \$\begingroup\$ @Sometowngeek, I believe I handled it in the play() function. if decision1 == decision2 is handling the tie. :) \$\endgroup\$ – BlahBlah Jul 7 '17 at 9:38
0
\$\begingroup\$
name = input("name?")
def players_input(player):
    while True:
    decision = input('Paper, Rock, or Scissors?: ')
    if decision == 'rock' or decision == 'paper' or decision == 'scissors':
        break
    else:
        print("Invalid choice! Try again!")
        continue
return '{} chooses {}'.format(player, decision)

So you won't be prompted to input your name every time you play again.

\$\endgroup\$

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.