4
\$\begingroup\$

Introduction.

It is commonly known that the worst form of torture is performing monotonous tasks ad nauseam. So I constructed this simple game I can give to my students when they misbehave.

Task.

The goal was to make a problem that could take in two numbers, and construct n distinct multiplication problems where the user is asked to answer what x * y is.

Challenge.

Where I struggled was with the part in cursive. How do I write a clear and readable code that produces distinct multiplication problems? I tried to do it with creating a sample of distinct numbers X, and then choose y such that y ≤ x where x is a member of X. Unsure if this is the best idea.

The code and some comments

  1. I choose to do import random instead of importing the modules as it is a bit clearer where the modules comes from
  2. As above, choose to include a short ___docstring___ more out of habit than necessity I guess..
  3. As mentioned above, do not really like my method for forcing the questions to be distinct.
  4. Tried to comply with pep8 for the long strings, looks a bit strange.

Here is the code

import random

QUESTIONS = 5
MIN_VAL = 2
MAX_VAL = 12


def arithmetic_game(val_1=MIN_VAL,
                    val_2=MAX_VAL,
                    num_of_problem=QUESTIONS):
    """ Ask the user a series of arithmetic questions
    and returns the number of correct answers.

    Args: val_1 (int): Defines the range of multiplication
    Args: val_2 (int): Defines the range of multiplication
    Args: number_of_problems (int): Defines the number of questions

    Return: correct_answers (int): number of correct math questions answered"""

    correct_answers = 0

    num_lst = random.sample(
            range(min(val_1, val_2), max(val_2, val_2)+1),
            num_of_problem)

    for num_1 in num_lst:
        num_2 = random.randint(2, num_1)
        print('\nWhat is {:d} times {:d}?'
              ''.format(num_1, num_2))
        answer = num_1*num_2

        try:
            if int(input()) == answer:
                print('Yes, the answer is {:d}'
                      ''.format(answer))
                correct_answers += 1
            else:
                print('No, the correct answer was {:d}'
                      ''.format(answer))
        except NameError:
            raise ValueError('I am sorry, you have'
                             'to write in a valid integer')

    return correct_answers


if __name__ == "__main__":

    correct_answers = arithmetic_game()
    print('\nYou got {:d} out of {:d} answers correct'
          ''.format(correct_answers, QUESTIONS))
\$\endgroup\$

2 Answers 2

1
\$\begingroup\$

I think you meant to write max(val_1, val_2) instead of max(val_2, val_2).

Long Strings

I personally think the PEP8's 79 characters-per-line requirement is too strict and is not practically optimal. Increase it to, for instance, 100 for this code snippet and you'll get a more readable and concise code with all of the used strings fitting on a single line.

Other Improvements

  • I would apply an "Extract Variable" refactoring method and define a tuple of report template strings, which would allow to bring the nestedness 1 level down:

    is_answer_correct = int(input()) == answer
    print(ANSWER_TEMPLATES[is_answer_correct].format(answer))
    correct_answers += is_answer_correct
    

    where ANSWER_TEMPLATES is defined as:

    ANSWER_TEMPLATES = (
        'No, the correct answer was {:d}',
        'Yes, the answer is {:d}'
    )
    

As far as generating unique random pairs for the problem, there is also this approach involving keeping track of seen pairs and keeping generating a next pair until it has not been seen.

\$\endgroup\$
-1
\$\begingroup\$
  • I think you need more functions. One to produce the random numbers, and one to get user input.
  • Personally I'd make your random number generator return all the numbers! This is as you can easily either slice the result, [:], or itertools.islice the result.
  • Follow PEP8 and don't be a hobgoblin. I'm always surprised when I remember that '{}' ''.format is allowed in Python, and format will act on the first string literal as well as the second.
  • I'd make a habit of keeping your try blocks as small as needed. This is so that you don't mask bugs. Whilst I don't think you'll be masking any here, it's a good habit to make. And so you could use else, or make only the small try except loop infinitely, so the user doesn't loose points for accidentally pressing a letter with a number.
  • I'm not sold by your need for the global constants. I'd make them 'magic literals'. However since they're mostly only used as default arguments, I personally wouldn't call them magic.
  • I'm not sold by val_1 and val_2, maybe minimum and maximum would be better? Python will also say they are the names when you go to use the function, and so will make more sense what is expected.

On the all, this is pretty small and I prefer your 'verbose' method of printing and more specifically adding to correct_answers than alecxe's. And so with the above minor changes I'd use something like:

import random
import itertools

QUESTIONS = 5
MIN_VAL = 2
MAX_VAL = 12


def generate_random_numbers(sample):
    sample = list(sample)
    for x in random.sample(sample, len(sample)):
        for y in random.sample(sample, len(sample)):
            yield (x, y)


def get_int_from_user():
    while True:
        try:
            return int(input())
        except NameError:
            raise ValueError('I am sorry, you have to write in a valid integer')


def arithmetic_game(val_1=MIN_VAL,
                    val_2=MAX_VAL,
                    num_of_problem=QUESTIONS):
    """
    Ask the user a series of arithmetic questions
    and returns the number of correct answers.

    Args: val_1 (int): Defines the range of multiplication
    Args: val_2 (int): Defines the range of multiplication
    Args: number_of_problems (int): Defines the number of questions

    Return: correct_answers (int): number of correct math questions answered
    """

    correct_answers = 0

    sample = range(min(val_1, val_2), max(val_1, val_2)+1)
    random_numbers = generate_random_numbers(sample, amount)
    random_numbers = itertools.islice(random_numbers, num_of_problem)

    for num_1, num_2 in num_lst:
        print('\nWhat is {:d} times {:d}?'.format(num_1, num_2))
        answer = num_1 * num_2
        if get_int_from_user() == answer:
            print('Yes, the answer is {:d}'.format(answer))
            correct_answers += 1
        else:
            print('No, the correct answer was {:d}'.format(answer))

    return correct_answers


if __name__ == "__main__":

    correct_answers = arithmetic_game()
    print('\nYou got {:d} out of {:d} answers correct'
          ''.format(correct_answers, QUESTIONS))
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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