6
\$\begingroup\$

For the following code, I have trouble improving it. I need to write a function getExpression(level, operator) which has one integer parameter representing a level and another string parameter representing the arithmetic operator. The function generates a string expression as well as the answer, based on the requirements for the level. Both the string expression and the result value are returned.

import random

def getExpression(level, operator):
    # Generate operands based on the level
    if operator == "+":
        # Addition
        if level == 1:
            num1 = random.randint(1, 9)  # Single digit operand
            num2 = random.randint(1, 9)
        elif level == 2:
            num1 = random.randint(10, 99)  # Two digit operand
            num2 = random.randint(10, 99)
        elif level == 3:
            num1 = random.randint(100, 999)  # Three digit operand
            num2 = random.randint(100, 999)
        elif level == 4:
            num1 = random.randint(1000, 9999)  # Four digit operand
            num2 = random.randint(1000, 9999)
        elif level == 5:
            num1 = random.randint(10000, 99999)  # Five digit operand
            num2 = random.randint(10000, 99999)
        
        # Return the expression and result
        result = num1 + num2
        expression = f"{num1} + {num2} = "
    
    elif operator == "-":
        # Subtraction
        if level == 1:
            num1 = random.randint(2, 9)  # Single digit operand
            num2 = random.randint(1, num1-1)  # Ensure num2 is smaller than num1
        elif level == 2:
            num1 = random.randint(10, 99)  # Two digit operand
            num2 = random.randint(1, num1-1)  # Ensure num2 is smaller than num1
        elif level == 3:
            num1 = random.randint(100, 999)  # Three digit operand
            num2 = random.randint(1, num1-1)  # Ensure num2 is smaller than num1
        elif level == 4:
            num1 = random.randint(1000, 9999)  # Four digit operand
            num2 = random.randint(1, num1-1)  # Ensure num2 is smaller than num1
        elif level == 5:
            num1 = random.randint(10000, 99999)  # Five digit operand
            num2 = random.randint(1, num1-1)  # Ensure num2 is smaller than num1
        
        # Return the expression and result
        result = num1 - num2
        expression = f"{num1} - {num2} = "
    
    elif operator == "*":
        # Multiplication
        if level == 1:
            num1 = random.randint(2, 9)  # Avoid starting with 1
            num2 = random.randint(2, 9)
        elif level == 2:
            num1 = random.randint(10, 99)
            num2 = random.randint(2, 9)
        elif level == 3:
            num1 = random.randint(100, 999)
            num2 = random.randint(2, 9)
        elif level == 4:
            num1 = random.randint(1000, 9999)
            num2 = random.randint(2, 9)
        elif level == 5:
            num1 = random.randint(10000, 99999)
            num2 = random.randint(2, 9)
        
        # Return the expression and result
        result = num1 * num2
        expression = f"{num1} * {num2} = "
    
    return expression, result
\$\endgroup\$
0

3 Answers 3

8
\$\begingroup\$

getExpression should be written get_expression by PEP 8, and should have type hints. The operator is a great candidate for being typed as a Literal.

The code is too repetitive. You need to DRY (don't repeat yourself) it up.

This code will be simplified by use of the modern match/case syntax.

Don't if on level (usually, other than one edge case). Just raise 10 to the power of the level.

Don't randint; these calls are better represented by randrange.

You need unit tests; I haven't shown any. Maybe I will later today.

All together,

import operator
import random
import typing


def get_expression(level: int, symbol: typing.Literal['+', '-', '*']) -> tuple[
    str,  # expression without result, i.e. 1 + 2 =
    int,  # expected value
]:
    if level < 1:
        raise ValueError(f'level must be at least 1, not {level}')

    low = 10**(level - 1)
    high = 10*low

    match symbol:
        case '+':
            op = operator.add
            a_low = low
        case '-':
            op = operator.sub
            if level == 1:
                a_low = low + 1
            else:
                a_low = low
        case '*':
            op = operator.mul
            if level == 1:
                a_low = 2
            else:
                a_low = low
        case _:
            raise ValueError(f'{symbol} is not a valid operator')

    a = random.randrange(a_low, high)

    match symbol:
        case '+':
            b_low = low
            b_high = high
        case '-':
            b_low = 1
            b_high = a
        case '*':
            b_low = 2
            b_high = 10

    b = random.randrange(b_low, b_high)
    expression = f'{a} {symbol} {b} = '
    expected = op(a, b)
    return expression, expected


def demo() -> None:
    print(get_expression(level=2, symbol='+'))
    print(get_expression(level=3, symbol='-'))
    print(get_expression(level=6, symbol='*'))


if __name__ == '__main__':
    demo()
('59 + 56 = ', 115)
('780 - 84 = ', 696)
('860709 * 7 = ', 6024963)
\$\endgroup\$
0
6
\$\begingroup\$

Documentation

The PEP 8 style guide recommends adding docstrings for functions. You can convert the function comment:

def getExpression(level, operator):
    # Generate operands based on the level

into a docstring:

def getExpression(level, operator):
    """ Generate operands based on the level """

It would also be good to add details about the input types and return types.

Naming

PEP 8 recommends snake_case for function names.

getExpression would be get_expression

Validation

Unless the code that calls your function makes sure that the inputs are valid, consider adding some simple checking to the function. For example, you should think about how to handle unexpected values passed to the level parameter. From the code, you expect it to be an integer between 1 and 5. If the function is passed an integer outside that range (like 6), the code exits with an error. The same is true if passed a floating-point value (2.1) or a string ("hello").

DRY

There is a lot of repeated code. For example, for the addition operator, you can take advantage of the relationship between the level and the upper and lower limits of the values passed to randint. For levels 2 and above, you can replace the hard-coded values (10, 99, 100, 999, etc.) with function calls, where you declare new functions:

def lower_limit(level):
    """ Return lower limit as a power of 10 """
    return 10**(level - 1)

def upper_limit(level):
    """ Return lower limit as a power of 10, minus 1 """
    return (10**level) - 1

This reduces the addition code to:

if operator == "+":
    # Addition
    num1 = random.randint(lower_limit(level), upper_limit(level))
    num2 = random.randint(lower_limit(level), upper_limit(level))
    
    # Return the expression and result
    result = num1 + num2
    expression = f"{num1} + {num2} = "

This eliminates much of the repetition, and it allows the code to scale to more values of level. These functions can also be used with the other operators.

\$\endgroup\$
2
  • 1
    \$\begingroup\$ Why do you add leading and trailing blanks to the docstring: """ Generate operands based on the level """? \$\endgroup\$ Commented Mar 17 at 14:53
  • 1
    \$\begingroup\$ @TimurShtatland: I think it is easier to read with the single space, don't you? \$\endgroup\$ Commented Mar 17 at 14:54
2
\$\begingroup\$

I'd keep it simple, mainly remove your level-based repetition. Compute basic low and high number for the level, then tweak where needed. The expression can use the given operator, no need to write it once for each case. The result could be computed with a single eval at the end as well, but each result = ... is painless and I like preparing each case's values (the operands and the result) in one place. For randint, I find random. redundant, so I'd avoid that.

from random import randint

def getExpression(level, operator):
    low = 10 ** (level-1)
    high = 10**level - 1

    if operator == "+":
        num1 = randint(low, high)
        num2 = randint(low, high)
        result = num1 + num2
    elif operator == "-":
        num1 = randint(max(low, 2), high)
        num2 = randint(1, num1-1)
        result = num1 - num2
    elif operator == "*":
        num1 = randint(max(low, 2), high)
        num2 = randint(2, 9)
        result = num1 * num2

    expression = f"{num1} {operator} {num2} = "
    return expression, result
\$\endgroup\$

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.