Number-guessing game in Python with a single level of abstraction

Question

I've read about keeping functions to the same level of abstraction, and it seems to make sense in all the examples. However, when translating to non-contrived programming, I'm still very unsure of my decisions. I'm especially unsure of where it applies. I wrote this guessing game to try to work on it, but it ended up feeling really clunky and redundant. How far should I go out of my way to flatten code? I read loops should ideally only be one function call, but that seems extreme. Is that really standard practice? If so, how would I refactor my main without needing a bunch of globals or really long parameter lists?

from random import randrange


DEFAULT_LO   = 1
DEFAULT_HI   = 100
HOT_DISTANCE = 10


def main():
    answer    = randrange(DEFAULT_LO, DEFAULT_HI+1)
    tries     = 1
    lastGuess = 0
    guess     = 0
    wasHot    = False

    won = False
    isFirstTry = True
    while (not won):
        lastGuess = guess
        guess = promptGuess()

        if validGuess(guess):
            if guess != answer:
                newDistance = abs(answer-guess)
                oldDistance = abs(answer-lastGuess)
                displayResult(newDistance, oldDistance, isFirstTry)

            else:
                displayWin(tries)
                won = True

            tries += 1
            isFirstTry = False


def promptGuess(lo=DEFAULT_LO, hi=DEFAULT_HI):
    guess = input('Enter a number ({}-{}): '.format(lo, hi))
    guess = int(guess)
    return guess


def validGuess(num, lo=DEFAULT_LO, hi=DEFAULT_HI):
    return (num >= lo and num <= hi)


def displayResult(newDistance, oldDistance, firstGuess):
    wasHot = (oldDistance <= HOT_DISTANCE)
    isHot = (newDistance <= HOT_DISTANCE)

    if firstGuess:
        displayHotCold(newDistance)

    elif newDistance is oldDistance: # Can happen with new answer if equally
        print('Same temperature!')   # far on opposite side of answer

    elif isHot:
        displayHotHotterCooler(newDistance, oldDistance, wasHot)

    else:
        displayColdWarmerColder(newDistance, oldDistance, wasHot)


def displayHotCold(newDistance):
    if newDistance <= HOT_DISTANCE:
        print('HOT!')

    else:
        print('Cold')


def displayHotHotterCooler(newDistance, oldDistance, wasHot):
    if not wasHot:
        print('HOT!!')

    elif newDistance < oldDistance:
        print('HOTTER!!!')

    else:
        print('COOLER')


def displayColdWarmerColder(newDistance, oldDistance, wasHot):
    if wasHot:
        print('COLD')

    elif newDistance < oldDistance:
        print('Warmer!')

    else:
        print('Colder')


def displayWin(tries):
    if tries is 1:
        print('WOW!! You got it on your first try!')
    else:
        print('GOOD JOB! You got it in {} tries.'.format(tries))


main()

Answer 1

When too many parameters are being passed around, object-oriented programming can help. But I don't think that that is your problem here. I think you have divided the work into functions suboptimally. Some advice:

• Document each function's purpose in a docstring.

  If you can't write a good docstring, then perhaps the function is poorly conceived. Can you tell me what the displayHotHotterCooler() function does? If you had to write a docstring for it, it might say:

  Assuming that a guess is hot, and that it is not the player's first guess, print 'HOT!!', 'HOTTER!!!', or 'COOLER' depending on how it rates relative to the previous guess.

  That's too complicated, I think. The function already has two preconditions on its applicability. Any attempt to describe it better would end up being more verbose than the code itself. Furthermore, the name displayHotHotterCooler doesn't really summarize the function's purpose. All these problems suggest that this is not a worthwhile function to define, because its purpose is not clear.

  In contrast, my display_hint() function below does have a clear summarizable purpose:

  Print a message hinting at the player's progress towards guessing the target number.

• Delegate, don't micromanage.

  promptGuess() could be smarter. It already knows what the valid range is, so why can't it perform the validation for you too? That would relieve main() from having to call validGuess(), and would eliminate one level of indentation from main().

  Part of not micromanaging is to let each function figure out what information it needs. Instead of passing three parameters newDistance, oldDistance, and firstGuess, consider passing the entire history of guesses instead, as a list. The called function can infer whether it's the first guess based on the length of the array. It can easily extract the last two elements, as needed. (Drawbacks are that you end up storing more state than is strictly necessary, and that you have to trust the called function not to mutate the list. But I think it's a worthwhile tradeoff for simplicity.)

Suggested solution

from random import randrange

def user_guesses(lo, hi):
    """
    Yield prompted guesses from the user, all verified to be integers
    between lo and hi (inclusive).
    """
    while True:
        guess = int(input('Enter a number ({}-{}): '.format(lo, hi)))
        if lo <= guess <= hi:
            yield guess

def display_win(diffs):
    """
    Print a congratulatory message for winning.
    """
    if len(diffs) == 1:
        print('WOW!! You got it on your first try!')
    else:
        print('GOOD JOB! You got it in {} tries.'.format(len(diffs)))

def display_hint(diffs, hot_distance):
    """
    Print a message hinting at the player's progress towards guessing
    the target number.

    diffs is a sequence of absolute differences between the guesses
    and the target number.  hot_distance is the threshold distance
    for considering a guess to be "hot".
    """
    is_hot = diffs[-1] <= hot_distance
    was_hot = diffs[-2] <= hot_distance if len(diffs) > 1 else None
    if was_hot is None:
        print('HOT!' if is_hot else 'Cold')
    elif diffs[-1] == diffs[-2]:
        print('Same temperature!')
    elif is_hot:
        print('HOT!!' if not was_hot else
              'HOTTER!!!' if diffs[-1] < diffs[-2] else
              'COOLER')
    else:
        print('COLD' if was_hot else
              'Warmer!' if diffs[-1] < diffs[-2] else
              'Colder')

def main(lo=1, hi=100):
    """
    Pick a random number between lo and hi (inclusive), and let the
    player guess the number.  Temperature hints are printed until
    the player guesses correctly.
    """
    hot_distance = (hi - lo + 1) // 10      # 10% of the range
    answer = randrange(lo, hi + 1)
    diffs = []
    for guess in user_guesses(lo, hi):
        diffs.append(abs(guess - answer))
        if guess == answer:
            display_win(diffs)
            return
        display_hint(diffs, hot_distance)

main()

Other remarks:

• Using the is operator to test for numeric equality is incorrect. It only happens to work for some smaller numbers.

  >>> 5 is -(-5)
  True
  >>> 1024 is -(-1024)
  False
  >>> 5 == -(-5)
  True
  >>> 1024 == -(-1024)
  True
  

• PEP 8, the official Python style guide, discourages extra spaces for the sake of aligning = signs. It also recommends lower_case_with_underscores instead of interCaps naming.

• Python support double-ended inequalities (lo <= num <= hi).

• Instead of sprinkling references everywhere to hard-coded constants, I often prefer to define default parameters.

• Flag variables, like won and isFirstTry, are almost always a sign that you can devise a better control structure. Just eliminate them from your code.

Answer 2

This is a very interesting program!

There are a couple of points to be made:

• Although Python doesn't enforce the Object-Oriented programming paradigm, this particular program should be written in Object-Oriented style
• I am not personally aware of any restrictions on the number of function calls a loop should have
• Try to avoid the is operator when just comparing values of two items. Here is a good post detailing when to use == and is
• Usually, it is good to compartmentalize your code (as you did in displayResult(), but the logic was a bit difficult to follow mainly due to the bad helper function names like displayColdWarmerColder().
• All Python executable programs should have an entry point. In your case, you should wrap your call to main() as shown below:

Example

if __name__ == '__main__':
    main()

You correctly identified the problem with the readability of your code: too many global variables. Instead, if you created an object, the object could contain those variables. Also, the object should handle the low-level game logic.

On a more positive note, your main while loop is actually very good in terms of abstraction, and it is very readable and understandable.

Here is my implementation of the game (I didn't test it much):

class GuessingGame:
    LO = 1
    HI = 100
    random = __import__('random')

    def __init__(self, lo=LO, hi=HI):
        self._lo = lo
        self._hi = hi
        self._hot_dist = (lo + hi) // 10
        self._num = self.random.randint(lo, hi)
        self._guesses = 0
        self._last_guess = None

    def get_range(self):
        '''Returns the lower and upper bounds'''
        return self._lo, self._hi

    def is_over(self):
        '''Checks if the game is over'''
        return self._last_guess == self._num

    def _display_temperature(self, num):
        '''Prints the temperature based on how close the guess was'''
        distance = abs(num - self._num)
        if self._last_guess is None:
            if distance <= self._hot_dist:
                print('Hot')
            else:
                print('Cold')
        else:
            old_dist = abs(self._last_guess - self._num)
            was_hot = old_dist <= self._hot_dist
            if old_dist == distance:
                print('Same temperature')
            elif distance <= self._hot_dist:
                if not was_hot:
                    print('Hot')
                elif distance < old_dist:
                    print('Hotter')
                else:
                    print('Cooler')
            else:
                if was_hot:
                    print('Cold')
                elif distance < old_dist:
                    print('Warmer')
                else:
                    print('Colder')

    def _update_guess(self, num):
        '''Updates the state of the game'''
        self._guesses += 1
        self._last_guess = num

    def process_guess(self, guess):
        '''Returns True if the guess was a valid guess'''
        try:
            num = int(guess)
        except Exception as e:
            return False

        self._display_temperature(num)
        self._update_guess(num)
        return True

    def display_result(self):
        '''Displays the state of the game'''
        if self.is_over():
            print('It took you {} tries.'.format(self._guesses))
        else:
            print('Last guess: {}'.format(self._last_guess))


if __name__ == '__main__':
    game = GuessingGame()

    while not game.is_over():
        valid_guess = False
        while not valid_guess:
            guess = input('Enter a number {}: \n'.format(game.get_range()))
            valid_guess = game.process_guess(guess)
            if not valid_guess:
                print("\'{}\' is an invalid query".format(guess.strip()))
    game.display_result()