Arithmetic Arranger I went too complex

I have seen a much easier version of this and understand it. For some reason when solving the problem/doing the project, I broke each part into a section and then just worked at it.

My question is: how can I get better at writing efficient code rather than complex code.

Here is my code for reference

#I made this wy harder than it should have been
def arithmetic_arranger(*problems):
    import re
    from textwrap import dedent
    #determines if True is present or not and assigns it
    if len(problems) >= 2:    tfresult = problems[1]
    else:   tfresult = False
    flist = list()
    slist = list()
    tlist = list()
    folist = list()

    #Rules (checks len of problems)
    if len(problems[0])>5:    return("Error: Too many problems.")
    #makes sure no division or multiplication
    for tasks in problems[0]:
        if "*" in tasks or "/" in tasks:    return("Error: Operator must be '+' or '-'.")
        #makes sure no letters
        if len(re.findall("[a-zA-Z]",tasks))>0:    return("Error: Numbers must only contain digits.")
        # splits task into each expression and checks length
        #variable assignment for for loop
        repeat = len(problems[0])
        for expression in tasks.split():
            if len(expression) > 4:    return("Error: Numbers cannot be more than four digits.")
            #END OF RULES
            #actual arranger
            if count<3: flist.append(expression)
            elif count<6: slist.append(expression)
            elif count<9: tlist.append(expression)
            elif count<12: folist.append(expression)

    alllists = [flist,slist,tlist,folist]
    iterationloop = 0
    for indlist in alllists:
        if iterationloop == repeat:  break
        #find how far +/- has to be indented
        f = int(indlist[0])
        s = int(indlist[2])
        op = str(indlist[1])
        if len(indlist[0])>=len(indlist[2]):
            opspace = len(indlist[0])
        elif len(indlist[0])<len(indlist[2]):
            opspace = len(indlist[2])
        # formats 1st and 2nd line accordingly (and adds plus to 2nd line)
        indlist[2] = indlist[2].rjust(opspace+1)
        indlist[1] = indlist[1] + indlist[2]
        indlist[0] = indlist[0].rjust(len(indlist[1]))
        indlist[2] = '-'*len(indlist[1])
        if op == "+":
        elif op == '-':
        indlist[3] = indlist[3].rjust(len(indlist[1]))
        rf.append("    ")
        rs.append("    ")
        rt.append("    ")
        if tfresult == True:
            rfo.append("    ")
        else: rfo=""

    #I feel there is a better quicker way to print the sums, this seems inefficient
    strf = ''.join(rf)
    strs = ''.join(rs)
    strt = ''.join(rt)
    strfo = ''.join(rfo)

    return dedent(f'''
         {strfo} ''')

Here is the simpler code

def arithmetic_arranger(problems, show = False):
  firstAnswer = ""
  operators = ""
  dashlines = ""
  secondAnswer = ""
  sumup = ""
  answer = ""
  sum = ""

  if(len(problems) > 5):
    return "Error: Too many problems."

  for problem in problems:
    #spliting the strings into a list
    firstNumber = problem.split(" ")[0]
    operators = problem.split(" ")[1]
    secondNumber = problem.split(" ")[2]

    # check the length of the number, max 4 digits
    if (len(firstNumber) > 4 or len(secondNumber) > 4):
      return "Error: Numbers cannot be more than four digits."

    # check the input as valid digits
    if not firstNumber.isnumeric() or not secondNumber.isnumeric():
      return "Error: Numbers must only contain digits."

    # check for the correct form of operators
    if (operators == '+' or operators == '-'):
      if operators == "+": 
        sum = str(int(firstNumber) + int(secondNumber))
      if operators == "-":
        sum = str(int(firstNumber) - int(secondNumber))
      return "Error: Operator must be '+' or '-'."

    length = max(len(firstNumber) , len(secondNumber)) + 2
    fisrtline = str(firstNumber).rjust(length)
    secondline = operators + str(secondNumber.rjust(length - 1))
    sltn = str(sum.rjust(length))
    dashline = ""
    for dash in range(length):
      dashline += "-"

    if problem != problems[-1]: 
      firstAnswer += fisrtline + '    '
      secondAnswer += secondline + '    '
      dashlines += dashline + '    '
      sumup += sltn + '    '
      firstAnswer += fisrtline
      secondAnswer += secondline 
      dashlines += dashline 
      sumup += sltn

  if show: 
    answer = firstAnswer + "\n" + secondAnswer + "\n" + dashlines + "\n" + sumup
    answer = firstAnswer + "\n" + secondAnswer + "\n" + dashlines    
  return answer

Hm… well that’s a hard question.
It’s about the way you think about problems and how you approach solving them.

It certainly comes with praxis, if you focus on it.
Like, if given a task you have to identify the problem and then think about how to break it down into steps for the computer. Before that, you can also think about how to turn it into simple tasks in general, before thinking about computers.

The “easy” solution for example identified that the solution has 4 lines of text and decided to go problem-by-problem via filling 4 seperate lines and only connecting them at the end.
[Edit: ok I think you did the same. Your problem is, that you worked with several lists which just add a extra transformation that isn’t needed]

This takes a lot of time thinking beforehand of how to approach the problem - on the data you have and the result you want.
Then it’s about how you could preprocess the data to extract all information and then how to re-arrange and transform the information in order to get to your result.

Two things that come to mind that could help getting to this point is both trying to recreate the easy solution WITHOUT looking. You know the concept, now go and make it yourself. You’ll force yourself to think about the new approach and get expirience which will further shape your process of thinking.

The other thing is going through your code and trying to figure out where you could shorten or improve things.
For example, the easy solution has a bunch of repeated code that could be trimmed down. So if you go through your code, maybe you find places where you wrote things that you could make shorter.

One comes to mind right on the spot, is how you create an array for the rows and then just .join() them at the end. But you never use anything exclusive to array, you just add data the same way you could directly add it to a string. So that’s like 15-20 lines of pointless code.
Or how for the final step you first make an empty string, then override it with .join() and then combine it in the f-string. You could literally just write the .join() into the f-string and trim down 2/3 of that section.

So that’s two approaches.
And a bonus tipp: if you gonna write numbers onto variables, just use numbers. Write list_1 instead of flist, row_3 instead of tr… because random abbreviations just make your code hard to read. Or if you number it anyway, put it into a list. Both are a lot easier to understand. And give it a level of order, which your concept of randomly adding the counter-letter in front or back (flist vs rf) is kinda lacking ^^°

Thankyou! I’m going to attempt the project from scratch again with what I now know from the simpler code (without looking). I’ll keep the array tips in mind, I definitely did that very inefficiently as I just googled how to join stuff onto lists and got “.join()”, so I used that.

thanks for taking the time to help me out, I appreciate it

I re-did it and managed to make it run 3* faster than the code that was already better than mine! Thankyou again for guidance and it really helped. here is code for reference

def arithmetic_arranger(problems, mode = False):
    if len(problems)>5:    return("Error: Too many problems.")
    #loops through each problem'3801 - 2'

    line_1st = str()
    line_2nd = str()
    line_3rd = str()
    line_4th = str()

    for problem in problems:
        #assigns each part of problem to var '3801' '-' '2'
        elements = problem.split()
        num_1st = elements[0]
        operator = elements[1]
        num_2nd = elements[2]
        #makes sure num_1st and num_2nd only numbers
        if num_1st.isdecimal()==False or num_2nd.isdecimal()==False: 
            return "Error: Numbers must only contain digits."
        #completes operation&checks for not"-,+"
        if operator == "+":
            answer = int(num_1st)+int(num_2nd)
        elif operator =="-":
            answer = int(num_1st)-int(num_2nd)
        else:    return "Error: Operator must be '+' or '-'."
        #checks number length
        num_max_len = max(len(num_1st), len(num_2nd))
        if num_max_len >4: return "Error: Numbers cannot be more than four digits."

        line_1st = line_1st + num_1st.rjust(num_max_len+2)+"    "
        line_2nd = line_2nd + operator + num_2nd.rjust(num_max_len+1)+"    "
        line_3rd = line_3rd + "-" *(num_max_len+2)+"    "
        line_4th = line_4th + str(answer).rjust(num_max_len+2)+"    "

    line_1st = line_1st[0:-4]
    line_2nd = line_2nd[0:-4]
    line_3rd = line_3rd[0:-4]
    line_4th = line_4th[0:-4]
    if mode == True:
        arranged = line_1st+"\n"+line_2nd+"\n"+line_3rd+"\n"+line_4th
    elif mode == False:
        arranged = line_1st+"\n"+line_2nd+"\n"+line_3rd

    return arranged

Good job ^^
Here are a few useful tips:

  1. If you just want to add a value to an existing object, you can use +=
  2. Python supports multiple-assignments, aka a, b = ["texta", "textb"]
  3. While not a huge deal, you can create arranged first and only use mode to add the final line if true - this saves repeated code and makes it obvious that mode only affects the final line.
  4. Also “if mode == True” is the same as “if mode”
1 Like

I’m a total beginner with python, so feel free to pass over this comment if anyone else says it’s no good. I’m doing that assignment too, and I found that after having worked out roughly how it might work (pseudocode thinking,) reading through the list of string methods and general functions on was helpful, as there were some pre existing functions there that are turning out to be efficient, in terms of code length and execution speed. But the main idea is to do lots of thinking before choosing the details of how you’ll solve each bit, like Jagaya said.


This topic was automatically closed 182 days after the last reply. New replies are no longer allowed.