Note that in my response I will be following the PEP8 naming conventions and so I might be referencing functions and variables that have been renamed from your original names.
Bugs
- You keep track of the number pairs being generated by storing in a list named
already_asked a tuple. But to see if a pair has already been generated, you are searching the list for a list of integers rather than a tuple of integers. Your search can never succeed. - In a couple of places you ask the user to enter an integer value. But if the user enters an invalid numeric, e.g. 'abc', your code will raise an exception. I would suggest that you create a function call
input_integer that will prompt the user with a user-supplied prompt until a valid value is entered. This function would profit by taking an optional range of integers (a tuple or list of two integers specifying a minimum and maximum value for the user's input). - If the user enters 0 for the number of questions to be asked, you put out an error message that the user cannot enter that value, but then you terminate the script. Instead use the
input_integer function mentioned above to ensure that the user supplies an answer that is at least 1 with some reasonable upper boundary. - As pointed out by STerliakov, there are only 25 * 25 = 625 possible pairs of randomly-generated numbers if the range for a random number includes 25 distinct values. So if the user wants to be asked more than 625 questions, the code will loop forever on the 626th question trying to generate a pair of numbers that haven't been seen before.
Choose More Meaningful Names for Functions and Variables
Function generateNumbers specifically generates a pair of integers and that this is so is explicitly assumed by the your code that calls the function. So better would be to rename it to something more descriptive, such as generate_integer_pair. You are also assigning the returned tuple of integers to a variable named genNum. This name could also be more descriptive, for example integer_pair.
Don't Repeat Yourself (DRY)
I don't believe you need to create a separate function that returns an integer within a specific range so that function generateNumbers does not have multiple calls to random.randint(-12, 12). I think it is sufficient to simply define "constants" such as FROM_INT, TO_INT = -12, 12 and use those constants instead of -12 and 12. This allows you to modify the range in a single place. But the code for this function can be simplified so that code to get the random pair does not need to be duplicated when a pair that has already been used before is detected.
A Small Inefficiency
In your function generateNumbers you have expressions such as genNum[0] specified multiple times. While this is not terribly inefficient, you could unpack the randomly generated tuple into a pair of simple variables once and then use those variables. It also makes it clear that generateNumbers is returning a tuple of two integers.
A Larger Inefficiency
Sequentially searching alreadyAsked to see if a random pair has already been used is less efficient than storing the pair as a tuple in a set instance.
Separation of Concerns
We would like function generateNumbers to generate a pair of integers that has not been previously generated yet you are relying on a client of this function to store all previously generated pairs. generateNumbers should be the one concerned with ensuring that generated pairs are remembered.
Make the Game "Importable"
Place the main logic in a function, for example multiplication_game, which can now be imported from another script.
Docstrings
Add these for the module and functions.
Type Hinting
Use this for function definitions.
Putting It All Together
"""This module provides a multiplication game.""" import random FROM_INT, TO_INT = -12, 12 # Define the range once! range_size = TO_INT - FROM_INT + 1 number_of_possible_pairs = range_size * range_size question_number = 1 num_correct = 0 already_asked = set() def generate_integer_pair() -> tuple[int]: """Generate a pair of integers in the closed interval [FROM_INT, TO_INT].""" #Checks if numbers have already been asked. while True: integer_pair = random.randint(FROM_INT, TO_INT), random.randint(FROM_INT, TO_INT) if integer_pair not in already_asked: break already_asked.add(integer_pair) return integer_pair def input_integer(prompt: str, valid_range=None) -> int: """Prompt the user for an integer value and optionally check to see if it within a certain range.""" while True: try: user_answer = int(input(prompt)) except ValueError: # Invalid input print('You must enter a valid integer value!') else: if valid_range is not None: min_value, max_value = valid_range if min_value <= user_answer <= max_value: return user_answer else: print(f'Your response must be an integer in the range [{min_value}, {max_value}]') else: return user_answer def multiplication_game(): """This is the game, which can be imported.""" global num_correct print("Welcome to the Multiplication Game!") num_questions = input_integer("How many Questions would you like to answer? ", (1, number_of_possible_pairs)) for question_number in range(1, num_questions + 1): integer_pair = generate_integer_pair() int1, int2 = integer_pair user_answer = input_integer(f'Question #{question_number}: {int1} x {int2}: ') answer = int1 * int2 if user_answer == answer: print("Correct!") num_correct += 1 else: print("Incorrect!") # An alternate approach suggested by Stef would be: # percent_correct = f'{num_correct*100/num_questions:.2f}'.strip('0').strip('.') percent_correct = round((num_correct / num_questions) * 100, 2) if percent_correct.is_integer(): percent_correct = int(percent_correct) print(f'Your score: {percent_correct}%') if __name__ == '__main__': multiplication_game()
Questions for Tomorrow
Python supports object-oriented programming. Where could it be put to good use? Can we create reusable classes that might be used in other applications? For example, how might we better encapsulate variable already_asked with the function generate_integer_pair that uses it?
