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'm a Python newbie and I'd like to know how I could clean this up/optimize it (but nothing too advanced, please).

As far as I know, it's working as it should, but if you see any errors then please point it out! This is a numerical guessing game with two symbols in front. You can pretty much see how it works by looking at the print messages.

from random import randint, choice    
import string
maxguesses = 11                       # Default number of max guesses
wins = 0                              
losses = 0                            
points = 0                            # Points accumulated
games = 0                             # Games played
numguesses = 0                        # Number of guesses
flag = True
allowed = list(string.digits + '+' + '*')
guesses = ''                                # Track wrong guesses, resets every game when formula is regenerated
letters = set()                             
partial = ''                                # Current progress of guesses

def genform():                              # Generate secret formula
    global secret, maxguesses               
    sym = choice(['++','**','+*','*+'])     # Pick operation symbols
    num = ''                                
    max = randint(3,9)                      # Set max between 3 & 9
    for i in range(max):                    # Range of max variable
        num = num + str(randint(0,9))       
    secret = sym + num                      # Assign secret formula
    maxguesses = len(secret)+2              # Max guesses based on length of formula
    return secret                           

def evaluate(formula):                      
    evaluated = ''
    s1,s2 = secret[:2]
    for i in range(2,len(secret)-1):        # Loop after two symbols
        evaluated = evaluated + secret[i]
        evaluated = (evaluated + s1) if not (i%2) else (evaluated + s2) # Place symbols appropriately
    evaluated = evaluated + secret[-1]      # Put together
    return evaluated                        

def takeguess(ch):
    global numguesses, wins, points, games, partial, guesses
    numguesses = numguesses + 1                                 # Add guess
    print 'Remaining Guesses: ' + str(maxguesses-numguesses)
    if (ch in partial) or (ch in guesses):                      # Check if already guessed
        print "\nYou've already guessed '%s'. Try again: " % ch
    elif ch not in secret:
        guesses = guesses + ch
        print 'Wrong, guess again.'
    elif ch in secret:                                          # If in formula
        letters.add(ch)                                         # Add guess to set
        print "\nGood guess, it's in the secret formula!"
        partial = ''.join([l if l in letters else '-' for l in secret])
        print 'Formula so far: ' + partial
        if partial == secret:                                   # If guesses are complete
            print 'You win!'
            wins = wins + 1
            points = points + 2
            games = games + 1
            bonus = raw_input("Evaluate the formula for 10 bonus points: ") # Ask bonus question
            if bonus == str(eval(evaluate(secret))):            # Calculate result
                points = points + 10
                print "That's correct! +10 points."
                print eval(evaluate(secret)) #REMOVE AFTER
                play_again()
            else:                                               # If bonus answer is wrong
                print "That's incorrect. The right answer is", eval(evaluate(secret))
                play_again()                                    

def play_again():                                   
    global numguesses, guesses, partial, letters, flag
    letters = set()                                             # Clear set
    if points < 2:                                              # Check points
        print "\nYou don't have enough points to play again.\nGames Played: " + str(games) + '\nPoints: ' + str(points)
    else:                                                       # Good to go
        ans = raw_input('Would you like to play again? [y/n] \n')
        if ans in ('yY'):
            numguesses = 0
            guesses = ''
            partial = ''
            play()
        elif ans in ('nN'):
            flag = False
            print '\nOkay, goodbye.\nWins: %s \nLosses: %s \nPoints: %s' % (wins, losses, points)
        else:
            print 'Invalid input.'
            play_again()

def play():                                                     
    global points, games, losses
    genform()                                                   # Generate new formula
    print 'Unsolved formula: ' + ('-'*len(secret))
    print 'You have %s guesses.' % maxguesses
    print secret #REMOVE AFTER
    while (numguesses < maxguesses) and flag:                   # Enough guesses and run OK
        guess = raw_input('\nEnter a guess: ')                  # Receive guess
        if guess not in allowed:                                # Check if valid
            print '\nInvalid guess, please enter a single digit, *, or +. Try again: '  
        elif partial != secret:                                 # Process guess
            takeguess(guess)
            print guesses # REMOVE AFTER
        if numguesses == maxguesses:                            # Guess limit reached
            points = points - 2
            games = games + 1
            losses = losses + 1
            print '\nSorry, you lose. The answer is: ' + secret
            play_again()                                        # Ask for replay
            return                                              

print 'Welcome to Numerical Hangman!\n'
play()
share|improve this question
4  
A good start would be not using globals - there is always a better way. In this case, pass more arguments and return more values. –  Lattyware Nov 11 '12 at 23:47
3  
My first suggestion is getting rid of the global variables. Is there any reason why you can't pass those as function parameters? If you need to keep more complicated state, then maybe you can create a class to represent it or use a dictionary. –  Pedro Romano Nov 11 '12 at 23:47
    
I believe the phrase is great minds think alike ;) –  Lattyware Nov 11 '12 at 23:48
    
I've only learned the very basics of global variables and I don't know about alternatives. Could an example be done for one of my global variables so I can visually see what is meant? –  user1790412 Nov 12 '12 at 0:19
add comment

1 Answer 1

up vote 5 down vote accepted
  • Not good:

    points = points - 2 # lots of this kind of code
    

    Good:

    points -= 2
    
  • Not good:

    allowed = list(string.digits + '+' + '*')
    

    Good:

    allowed = string.digits + '+*' #You don't need list in this case
    
  • Not good:

    max = randint(3,9)
    

    Good:

    _max = randint(3,9) # max is a function in standard namespace
    
  • Not good:

    print 'Remaining Guesses: ' + str(maxguesses-numguesses)
    

    Good:

    print 'Remaining Guesses: {}'.format(maxguesses-numguesses)
    
  • Not good:

    if bonus == str(eval(evaluate(secret))):   # what do you mean here?
    
  • Not good:

    def evaluate(formula):  
    ...
    

    Your local var formula was not used in this function. Instead of this, you're using global secret.

  • Not good:

    genform()  
    ...  
    return secret
    

    You're returning secret in this function, but you don't use it in your app. secret is global anyway.

That's a briefly view. Please read Google’s Python style guide.

share|improve this answer
1  
Some of the information at that link is outdated. For example, they advocate using % for string formatting when people should be using str.format() now. (actually, that is the only problem I saw) –  Matt Nov 13 '12 at 14:41
    
Thanks for the tip. updated –  Nikles Nov 14 '12 at 8:05
    
+1 for good/not good format and # comments. –  avip Nov 14 '12 at 9:09
    
string.digits.join creates lots of duplicate copies of '+*' in the allowed string which, while not erroneous, is probably not what you meant. You also probably want it to be a set. –  raylu Dec 14 '12 at 4:34
add comment

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.