Solution to arithmetic arranger from freecodecamp

Question

I tried my hand at the freecodecamp python algorithm and I came up with a solution for the first question but need help refactoring it. Here is the arithmetic arranger question click here to access the question and the detailed question can be found below:

Students in primary school often arrange arithmetic problems vertically to make them easier to solve. For example, "235 + 52" becomes:

  235
+  52
-----

Create a function that receives a list of strings that are arithmetic problems and returns the problems arranged vertically and side-by-side. The function should optionally take a second argument. When the second argument is set to True, the answers should be displayed.

Example

Function Call:

arithmetic_arranger(["32 + 698", "3801 - 2", "45 + 43", "123 + 49"])

Output:

   32      3801      45      123
+ 698    -    2    + 43    +  49
-----    ------    ----    -----

Function Call:

arithmetic_arranger(["32 + 8", "1 - 3801", "9999 + 9999", "523 - 49"], True)

Output:

  32         1      9999      523
+  8    - 3801    + 9999    -  49
----    ------    ------    -----
  40     -3800     19998      474

Rules

The function will return the correct conversion if the supplied problems are properly formatted, otherwise, it will return a string that describes an error that is meaningful to the user.

• Situations that will return an error:
  • If there are too many problems supplied to the function. The limit is five, anything more will return: Error: Too many problems.
  • The appropriate operators the function will accept are addition and subtraction. Multiplication and division will return an error. Other operators not mentioned in this bullet point will not need to be tested. The error returned will be: Error: Operator must be '+' or '-'.
  • Each number (operand) should only contain digits. Otherwise, the function will return: Error: Numbers must only contain digits.
  • Each operand (aka number on each side of the operator) has a max of four digits in width. Otherwise, the error string returned will be: Error: Numbers cannot be more than four digits.

If the user supplied the correct format of problems, the conversion you return will follow these rules:

• There should be a single space between the operator and the longest of the two operands, the operator will be on the same line as the second operand, both operands will be in the same order as provided (the first will be the top one and the second will be the bottom.
• Numbers should be right-aligned.
• There should be four spaces between each problem.
• There should be dashes at the bottom of each problem. The dashes should run along the entire length of each problem individually. (The example above shows what this should look like.)

The solution can be found below:

import operator
ops = {"+": operator.add, "-": operator.sub, "*": operator.mul}


def arithmetic_arranger(problems, solver=False):
    # Check problems does not exceed the given max(5)
    if len(problems) > 5:
        return "Error: Too many problems."
    toptier = ""
    bottomtier = ""
    lines = ""
    totals = ""
    for n in problems:
        fnumber = n.split()[0]
        operator = n.split()[1]
        snumber = n.split()[2]

        # Handle errors for input:
        if operator != "+" and operator != "-":
            return "Error: Operator must be '+' or '-'."
        if not fnumber.isdigit() or not snumber.isdigit():
            return "Error: Numbers must only contain digits."
        if len(fnumber) > 4 or len(snumber) > 4:
            return "Error: Numbers cannot be more than four digits"

        # Get total of correct function
        total = ops[operator](int(fnumber), int(snumber))
        # Get distance for longest operator
        operatorDistance = max(len(fnumber), len(snumber)) + 2

        snumber = operator + snumber.rjust(operatorDistance - 1)
        toptier = toptier + fnumber.rjust(operatorDistance) + (4 * " ")
        bottomtier = bottomtier + snumber + (4 * " ")
        lines = lines + len(snumber) * "_" + (4 * " ")
        totals = totals + str(total).rjust(operatorDistance) + (4 * " ")
    if solver:
        print(toptier)
        print(bottomtier)
        print(lines)
        print(totals)


if __name__ == "__main__":
    arithmetic_arranger(["32 + 698", "3801 - 2", "45 + 43", "123 + 49"])

Answer 1

Use PEP484 type hints.

The too-many-problems and too-many-digits constraints are not good ones, but whatever: we'll keep them because the problem asks so.

You call split() three times when you should only call it once and tuple-unpack to three substrings.

You should not make literal comparisons of the operator to potential strings, nor should you include + and - verbatim in the error message. Instead, derive these from your ops dictionary (you adding it was a good idea!)

Where possible, pass validation problems from subroutines to your main routine using exceptions.

Your len(fnumber) is technically not correct if they expand the problem to allow negative numbers. The minus sign constitutes another character. They want you to limit digit count, not character count. I think a safer check would be comparing to abs() >= 1e4.

fnumber and snumber (short for "first" and "second") are non-obvious, and should use something else: number_1, or more easily just x and y since it's pretty obvious what's going on.

One potential refactor could look like:

• Create a simple class representing a problem
• On the class, have a parse that loads a class instance from a string
• If parse fails with a ValueError, the string isn't parseable: bail.
• If parse succeeds but validate() fails, the string is parseable but invalid: bail.
• Write a formatting method that returns a tuple of lines for one problem.
• In your upper method, zip the lines of all of the problems together, join the lines with \n and the groups among each line with however many spaces you want (looks like 4).

Suggested

import operator
from typing import Sequence, NamedTuple, Literal

ops = {"+": operator.add, "-": operator.sub}


class Problem(NamedTuple):
    x: int
    y: int
    op: Literal['+', '-']

    @classmethod
    def parse(cls, s: str) -> 'Problem':
        x, op, y = s.split()

        for n in (x, y):
            if not n.isdigit():
                raise ValueError('Error: Numbers must only contain digits.')

        return cls(x=int(x), y=int(y), op=op)

    def validate(self) -> None:
        for n in (self.x, self.y):
            if abs(n) >= 1e4:
                raise ValueError('Error: Number cannot be more than four digits.')

        if self.op not in ops:
            raise ValueError(
                'Error: Operator must be '
                + ' or '.join(f"'{o}'" for o in ops.keys())
            )

    def format_lines(self, solve: bool = False) -> tuple[str, ...]:
        longest = max(self.x, self.y)
        width = len(str(longest))
        lines = (
            f'{self.x:>{width + 2}}',
            f'{self.op} {self.y:>{width}}',
            f'{"":->{width+2}}',
        )
        if solve:
            lines += (
                f'{self.answer:>{width+2}}',
            )
        return lines

    @property
    def answer(self) -> int:
        return ops[self.op](self.x, self.y)


def arithmetic_arranger(problem_strings: Sequence[str], solve: bool = False) -> None:
    if len(problem_strings) > 5:
        print('Error: Too many problems.')
        return

    try:
        problems = [Problem.parse(s) for s in problem_strings]
        for problem in problems:
            problem.validate()
    except ValueError as e:
        print(e)
        return

    lines = zip(*(p.format_lines(solve) for p in problems))
    print(
        '\n'.join(
            '    '.join(groups) for groups in lines
        )
    )


if __name__ == "__main__":
    arithmetic_arranger((
        "32 + 698",
        "3801 - 2",
        "4 + 4553",
        "123 + 49",
        "1234 - 9876"
    ), solve=True)