Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I was eventually going to add another computer and more options to this (i.e., Rock-paper-scissors-lizard-Spock) but I wanted to make sure I was using the best practice for this type of game. Any help would be appreciated.

# The traditional paper scissors rock game

import os
def clear():
    os.system("clear")
clear()
print ("\n\nPaper, Rock, Scissors Game -(Best of five games)")
x = 0 ;  l = 0 ;  w = 0 ; d = 0 ; lt = 0 ; wt = 0 ; dt = 0
while x < 5:
  x = x + 1
  import random
  class Computer:
         pass
  comp_is = Computer()
  comp_is.opt = ('r','p','s')
  comp_is.rand = random.choice(comp_is.opt)

  if comp_is.rand == 'r':
            comp  = 'rock'
  elif comp_is.rand == 'p':
            comp  = 'paper'
  else:
        comp  = 'scissors'

  class Human:
       pass
  human_is = Human
  print
  human_is.player = raw_input(' Enter your choice of\n   r\
 for rock\n   p for paper or\n   s for scissors ... ')
  print

  class Result:
     pass
  Result_is = Result
  if comp_is.rand == human_is.player:
    print ("draw - computer chose ",  comp)
    print
    d = d + 1
    dt = dt + 1
  elif comp_is.rand == 'r' and human_is.player == 'p':
      print ("  player beats computer -computer chose ",  comp)

      print
      w = w + 1
      wt = wt + 1
  elif comp_is.rand == 'p' and human_is.player == 's':
      print ("  computer chose ",  comp)
      print ("  player beats computer-because scissors cuts paper")
      print ()
      w = w + 1
      wt = wt + 1
  elif comp_is.rand == 's' and human_is.player == 'r':
     print (" computer chose ", comp) 
     print (" player beats computer-because rock breaks scissors")
     w = w + 1
     wt = wt + 1
  else :
     print ("   computer wins - computer chose  ", comp)
     print
     l = l + 1
     lt = lt + 1

  if x == 5:
    print ()
    print ()   
    print ("  games  won ... ",  w)
    print ("  games lost ... ",  l)
    print ("  games drawn ... ",  d)
    print ()
    print ("  Running total overall of games won ... ", wt)
    print ("  Running total overall of games lost ... ", lt)
    print ("  Running total overall of games drawn ... ", dt)
    print ()
    w = 0 ; l = 0 ; d = 0
    again = input('Do you want to play again y for yes, n for no ..  ')
    if again == 'y':
       x = 0
    else:
      print 
      if lt > wt:
         print ("You are a miserable loser,\nYou have lost more than you have won,\nPoor show indeed ")
         print ('finish')
share|improve this question
    
I started analyzing the improved version of your question, but it has been removed for some reason. Line-by-line analysis is gone, but I wrote my version of a program: codereview.stackexchange.com/questions/46508/…. –  Ruslan Osipov Apr 6 at 23:14

1 Answer 1

up vote 7 down vote accepted

There is much to improve. I recommend you read PEP 8, the official Python style guide. It includes many important tips like:

  • Use consistent 4-space indentation

  • Multiple statements on the same line are discouraged

Furthermore:

  • You should use words instead of single letters for your variable names. All those variables like x, l, w, d, lt, wt, dt aren't self-explaining. What is their purpose? Instead: count_rounds, count_losses, count_wins, count_draws, ….

  • Your usage of object oriented features is extremely weird. Get rid of all classes for now.

  • Instead of x = x + 1 write x += 1.

  • Your code is really complicated because it munges together the user interface (prompting the user for choices, displaying results) with the actual logic of your program. Put the logic into separate functions, e.g.

    def beats(choice_a, choice_b):
        if choice_a == 'rock' and choice_b == 'scissors':
            return 'smashes'
        if choice_a == 'scissors' and choice_b == 'paper':
            return 'cuts'
        if choice_a == 'paper' and choice_b == 'rock':
            return 'wraps'
        else:
            return None
    

    This could be used as

    def result_string(computer, player):
        verb = beats(computer, player)
        if verb:
            return "computer beats player because %s %s %s" % (computer, verb, player)
        verb = beats(player, computer)
        if verb:
            return "player beats computer because %s %s %s" % (player, verb, computer)
        return "draw"
    

    which in turn could be used as print(result_string(computer_choice, player_choice)).

Please try to fix these issues and to clean up your code, then come back and ask a new question for a second round of review.

share|improve this answer
    
Will fix up the code and come back for second round of review. –  techteej Apr 6 at 21:35
    
here is my updated program –  techteej Apr 6 at 21:59
1  
@TjF As far as I can see, you only improved variable names. That is good, but I mentioned many more issues like indentation and helper functions. Any review on your updated code will mention those as well, which is unnecessary. Consider doing more substantial improvements. –  amon Apr 6 at 22:05
    
I was sort of confused by this. Wondering if you could help me a little further on that? –  techteej Apr 6 at 22:06
1  
@TjF your code has two concerns: deciding who won, and printing out the result. Combining these two is inelegant, it is better to put each concern into a seperate function. So my above answer suggested one function to find out which move one (and will return a verb describing how the move won), plus one function to format the message. The main input loop doesn't have to do anything besides printing out the actual result. This will make it easier to extend your program later. –  amon Apr 6 at 22:11

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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