3
\$\begingroup\$

I'm looking for tips on how to clean up this code and/or make it more efficient.

Keep in mind I am a seriously new programmer and if it is too advanced it's likely to go over my head.

import random
cscore = 0
pscore = 0
print 'I would like to play a game. \nThe name of the game is Rock, Paper, Scissors.'
rest = 'y'
while (rest == 'y'):
    pick = ['r', 'p', 's']
    b = (random.choice(pick))
    a = raw_input ('\nFirst Letter ')
    if a == 'r' or a == 'R':
        a = 'Rock'
    elif a == 'p' or a == 'P':
        a = 'Paper'
    elif a == 's' or a == 'S':
        a = 'Scissors'

### assign name to computer's answer
    if b == 'r':
        b = 'Rock'
    elif b == 'p':
        b = 'Paper'
    elif b == 's':
        b = 'Scissors'    
    print 'You picked ' + a + '\nAnd I picked ' + b



##### compare picks
    if a == 'Rock' and b  == 'Rock':
        print 'Silly, we both picked Rock, \nthat means we tied.'
    elif a == 'Rock' and b == 'Paper':
        print 'Yes! Paper smothers Rock, therefore, I win.'
        cscore = cscore + 1
    elif a == 'Rock' and b == 'Scissors':
        print 'Crap, Rock smashes Scissors. You win.'
        pscore = pscore + 1
    elif a == 'Paper' and b  == 'Paper':
        print 'Silly, we both picked Paper, \nthat means we tied.'
    elif a == 'Paper' and b == 'Scissors':
        print 'Yes! Scissors cut Paper, therefore, I win.'
        cscore = cscore + 1
    elif a == 'Paper' and b == 'Rock':
        print 'Crap, Paper smothers Rock. You win.'
        pscore = pscore + 1
    elif a == 'Scissors' and b  == 'Scissors':
        print 'Silly, we both picked Scissors, \nthat means we tied.'
    elif a == 'Scissors' and b == 'Rock':
        print 'Yes! Rock smashes Scissors, therefore, I win.'
        cscore = cscore + 1
    elif a == 'Scissors' and b == 'Paper':
        print 'Crap, Scissors cut Paper. You win.'
        pscore = pscore + 1
    print '\nThe score is now Computer ' + str(cscore) + ' Human ' +     str(pscore)
    rest = raw_input ('\nWould you like to play again? y or n ')
print '\nThank you for playing. The final score was Computer ' + str(cscore) + ' Human ' + str(pscore)
if cscore > pscore:
    print 'Ha!, I won and you lost.'
elif cscore == pscore:
    print 'We tied in the overall score, you are a worthy opponent.'
elif cscore < pscore:
    print 'You beat me, you low down mangy son of a gun. You must have cheated.'
\$\endgroup\$
1
  • \$\begingroup\$ See my answer here for implementation details and some remarks on variable names. \$\endgroup\$ May 20 '15 at 1:54
4
\$\begingroup\$
  • You used a massive number of if and elif statments, I suggest using Math

  • You used no functions, I suggest using some.

  • You are repeating yourself very much, I suggest to say things once and only once.

  • You are mixing logic and user interface, I suggest not to.

  • You are not automatically testing your code, changing it may break it.

My implementation does not count wins and losses and is a bit hard to follow but was developed following the above principles that I believe make good code:

import doctest
import random

ROCK = 0
PAPER = 1
SCISSORS = 2
SIGNS = ['ROCK', 'PAPER', 'SCISSORS']

LETTER_TO_RPS = {
    'r':ROCK,
    'p':PAPER,
    's':SCISSORS
    }

def rps_compare(a, b):
    """
    >>> rps_compare(ROCK, SCISSORS)
    'WIN'
    >>> rps_compare(PAPER, SCISSORS)
    'LOSS'
    """
    if a == b: return 'TIE'
    return 'WIN' if (a - b) % 3 == 1 else 'LOSS'

def rps_round(letters_map=LETTER_TO_RPS):
    user_choice = LETTER_TO_RPS[raw_input("[R]ock, [P]aper or [S]cissors? ").lower()]
    cpu_choice = random.choice([ROCK,PAPER,SCISSORS])
    print("I chose {}.".format(SIGNS[cpu_choice]))
    return rps_compare(user_choice, cpu_choice)

def rps_game(welcome_message, letters_map=LETTER_TO_RPS):
    print(welcome_message)
    while True:
        print("This is a {}".format(rps_round()))


if __name__ == "__main__":
    doctest.testmod()
    rps_game('Hello')
\$\endgroup\$
6
  • \$\begingroup\$ Ok, thanks for pointing out these things. As I'm looking through your code I have a couple questions, if you don't mind answering @Caridorc . First: on the user_choice input does the .lower() force the user input to lower case so you don't have to worry about Upper or Lower? 2. How does the script determine which input is a and which is b to compare? \$\endgroup\$
    – Muertes
    May 19 '15 at 19:07
  • \$\begingroup\$ @Muertes So, '.lower()` is as you guessed is to allow both upper and lower_case input, -- The code determines a and b by the order they are passed into the rps_compare function, the first is a, the second is b \$\endgroup\$
    – Caridorc
    May 19 '15 at 19:12
  • \$\begingroup\$ I would keep doctest out of the code and instead run it from the command line with python -m doctest if necessary. \$\endgroup\$
    – mkrieger1
    May 20 '15 at 12:24
  • \$\begingroup\$ @mkrieger1 that's possible but doc test means both documentation and testing, if you put it outside you lose the documentation part. \$\endgroup\$
    – Caridorc
    May 20 '15 at 12:54
  • \$\begingroup\$ I didn't mean to remove the doctests from the code, but to remove the lines import doctest and doctest.testmod(). Instead, in order to run the doctests, the script can be run from the command line with python -m doctest rps.py. That way, they are only run if it is desired and not always. \$\endgroup\$
    – mkrieger1
    May 20 '15 at 14:06

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.