Evan Emolo
Evan Emolo

Reputation: 1670

Python: Refactor code to remove global variable

I am currently using a global variable in my code called correct. Considering globals are frowned upon, is there a better way to set up my code to "protect" the global variable?

from random import randint
from random import choice

lower  = int(raw_input("Enter a lower integer constraint: "))
higher = int(raw_input("Enter a higher integer constraint: "))

correct = 0


def gen_randoms(lower, higher):
    integers = list()
    for x in xrange(4):
        rand_int = randint(lower, higher)
        integers.append(rand_int)
    return integers


def gen_equation(integers):
    nums = map(str, integers)
    operators = ['*', '+', '-']
    equation = 'num op num op num op num'
    while 'op' in equation:
        equation = equation.replace('op', choice(operators), 1)
    while 'num' in equation:
        equation = equation.replace('num', choice(nums), 1)
    print equation
    return equation


def evaluate(equation):
    answer = eval(equation)
    print answer
    return answer


def compare_answers(gen_answer, game):
    global correct
    user_answer = int(raw_input("What is the answer? "))
    if user_answer == gen_answer:
        correct += 1
        print 'Correct!'
        print 'Current streak: %s' % str(correct)
        game()
    else:
        print 'Incorrect!'
        correct = 0
        game()


def game():
    nums = gen_randoms(lower, higher)
    this_equation = gen_equation(nums)
    gen_answer = evaluate(this_equation)
    compare_answers(gen_answer, game)


game()

Upvotes: 6

Views: 2598

Answers (6)

Crowman
Crowman

Reputation: 25908

I'd probably do it something like this:

#!/usr/bin/python

"""Equation solving game."""

from random import randint
from random import choice


def gen_randoms(lower, higher):

    """Generates four random numbers between provided bounds."""

    integers = [randint(lower, higher) for x in range(4)]
    return integers


def gen_equation(integers):

    """Generates a random equation from four provided integers."""

    nums = [str(i) for i in integers]
    operators = ['*', '+', '-']
    equation = 'num op num op num op num'
    while 'op' in equation:
        equation = equation.replace('op', choice(operators), 1)
    while 'num' in equation:
        equation = equation.replace('num', choice(nums), 1)
    return equation


def evaluate(equation):

    """Evaluates an equation."""

    return eval(equation)


def main():

    """Main game function."""

    lower = int(raw_input("Enter a lower integer constraint: "))
    higher = int(raw_input("Enter a higher integer constraint: "))
    nums = gen_randoms(lower, higher)
    streak = 0

    while True:
        this_equation = gen_equation(nums)
        print this_equation

        user_answer = raw_input("What is the answer? ('Q' to quit) ")

        if user_answer.lower()[0] == 'q':
            break

        gen_answer = evaluate(this_equation)
        print 'The answer was: %d' % gen_answer

        if gen_answer == int(user_answer):
            streak += 1
            print 'Correct!'
            print 'Current streak: %d' % streak
        else:
            streak = 0
            print 'Incorrect!'


if __name__ == "__main__":
    main()

A few comments:

  • Each function should generally only do one thing, so if a function evaluates an equation, it's usually better to not have it print the equation, too.
  • Figuring out where the variables ought to go becomes a lot easier when you do this, since you don't need to pass them around so much as you have to when each function does several different things.
  • The main game logic here is sufficiently straightforward that you don't really need to break it out of your main game() function (or main() function, in my example) so much, it doesn't overcomplicate things to leave it in there. If you wanted to do some more error checking (e.g. seeing if the user entered an invalid number, and you want to go back and ask for more input if so) then you might want to break it out some more.

Upvotes: 3

Burhan Khalid
Burhan Khalid

Reputation: 174624

Collect your results in the caller, and then print the count. This version gets rid of your other globals, lower and higher.

def compare_answers(gen_answer):
    user_answer = int(raw_input("What is the answer? "))
    return user_anser == gen_answer

def game():

    correct = 0
    play_again = 'Y'

    while play_again.lower() == 'y':
        lower  = int(raw_input("Enter a lower integer constraint: "))
        higher = int(raw_input("Enter a higher integer constraint: "))
        nums = gen_randoms(lower, higher)
        this_equation = gen_equation(nums)
        gen_answer = evaluate(this_equation)
        if compare_answers(gen_answer):
           correct += 1
           print('You were right! Your winning streak is {0}'.format(correct))
        else:
           print('Sorry, your answer was wrong. :(')
           correct = 0

        play_again = raw_input('Would you like to play again? (Y/N): ')

Upvotes: 0

Evan Emolo
Evan Emolo

Reputation: 1670

I ended up taking @JoelCornett's suggestion and creating a class:

from random import randint
from random import choice

class Math_Problem_Generator(object):

    def __init__(self):
        self.lower = int(raw_input("Enter a lower integer constraint: "))
        self.higher = int(raw_input("Enter a higher integer constraint: "))
        self.correct = 0
        self.game(self.correct)


    def gen_randoms(self, lower, higher):
        integers = list()
        for x in xrange(4):
            rand_int = randint(lower, higher)
            integers.append(rand_int)
        return integers


    def gen_equation(self, integers):
        nums = map(str, integers)
        operators = ['*', '+', '-']
        equation = 'num op num op num op num'
        while 'op' in equation:
            equation = equation.replace('op', choice(operators), 1)
        while 'num' in equation:
            equation = equation.replace('num', choice(nums), 1)
        print equation
        return equation


    def evaluate(self, equation):
        answer = eval(equation)
        print answer
        return answer


    def compare_answers(self, gen_answer):
        user_answer = int(raw_input("What is the answer? "))
        if user_answer == gen_answer:
            self.correct += 1
            print 'Correct!'
            print 'Current streak: %s' % str(self.correct)
            self.game(self.correct)
        else:
            print 'Incorrect!'
            self.correct = 0
            self.game(self.correct)


    def game(self, correct):
        nums = self.gen_randoms(self.lower, self.higher)
        this_equation = self.gen_equation(nums)
        gen_answer = self.evaluate(this_equation)
        self.compare_answers(gen_answer)
        self.game()


Math_Problem_Generator()

Upvotes: 0

Bleeding Fingers
Bleeding Fingers

Reputation: 7129

  1. remove the global correct from compare_answers
  2. set game to take in a keyword argument correct with default value 0. def game(correct =0)
  3. set compare_answers to take a third argument correct
  4. when calling game from compare_answers pass in correct

i.e.:

def compare_answers(gen_answer, game, correct):
    user_answer = int(raw_input("What is the answer? "))
    if user_answer == gen_answer:
        correct += 1
        print 'Correct!'
        print 'Current streak: %s' % str(correct)
        game(correct)
    else:
        print 'Incorrect!'
        correct = 0
        game(correct)


def game(correct = 0):
    nums = gen_randoms(lower, higher)
    this_equation = gen_equation(nums)
    gen_answer = evaluate(this_equation)
    compare_answers(gen_answer, game, correct)


game()

Upvotes: 0

Nayuki
Nayuki

Reputation: 18533

Add this:

def game():
    correct = 0
    while True:
        nums = gen_randoms(lower, higher)
        this_equation = gen_equation(nums)
        gen_answer = evaluate(this_equation)
        user_answer = int(raw_input("What is the answer? "))
        if user_answer == gen_answer:
            correct += 1
            print 'Correct!'
            print 'Current streak: %s' % str(correct)
        else:
            print 'Incorrect!'
            correct = 0

Then delete these pieces of old code: game(), compare_answers(), and the global variable correct.

Upvotes: 0

Jiminion
Jiminion

Reputation: 5168

something like this:

def game(correct=0):    
    nums = gen_randoms(lower, higher)  
    this_equation = gen_equation(nums)  
    gen_answer = evaluate(this_equation)  
    correct = compare_answers(gen_answer, game, correct)  
    game(correct)  

Upvotes: 1

Related Questions