1
\$\begingroup\$

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"])

\$\endgroup\$
1
  • \$\begingroup\$ Thanks i have added the question \$\endgroup\$
    – Xanik
    May 15 at 19:32

1 Answer 1

1
\$\begingroup\$

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)
\$\endgroup\$
2
  • 1
    \$\begingroup\$ Thanks for this! breaks it down into different components and more readable by making the validator and parser into a func to be called. I also liked your approach to raising an err if a wrong validator was inputed! \$\endgroup\$
    – Xanik
    May 15 at 20:46
  • \$\begingroup\$ Just a few comments intended in a constructive spirit to help @Xanik; not necessarily encouraging you to modify your useful answer. (1) Name ops as a proper constant: OPS (or even OPERATIONS for more clarity). (2) If we're doing validation of the text input, we should probably handle possible failures here: x, op, y = s.split(). (3) Seems more practical and natural to validate everything during parsing, eliminating the need for a separate validate() method. It's certainly not uncommon for validation to happen during parsing in other contexts. \$\endgroup\$
    – FMc
    May 16 at 0:05

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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