4
\$\begingroup\$

So I made this script for my calculator, so that I can just write whatever formula I want and the calculator would just ask for the missing pieces to do the calculations.

I made it so that it would be very easy to fill in, I just go to the end of the list, give it a name and the formula. That's it!

The working of it is as followed:

  1. User choses which topic/name to use
  2. The script checks how many formulas are under that name if it's greater than 1 he needs to make another choice for which formula to run
  3. The script will start searching for the variables that are needed to be filled in by the user and ask the user to give them a value, to achieve this I used eval() and exec(). (It is a calculator used by me so I don't have security concerns and the runtimes were pretty great too)
  4. It will print the answer

What could I have done better and if it was the right decision to make use of eval() and exec() functions? I needed a fast and a less complex way to just get what I want and they just felt like the perfect solution.

Here's the code:

from math import *

SWIDTH = 42

# Accessing shapes will happen by their index number
# The list is formed in the following structure [Name or "Mother", formula 1, formula 2, ...]
list_shapes = [
    ["cilinder", "pi*r^2*h", "2*r*h", "2*r*h + 2*pi*r^2"],
    ["kegel", "1/3*pi*r^2*h", "pi*r*a", "pi*r*a + pi*r^2"],
    [
        "afgeknokte kegel",
        "1/3*pi*h*((r_1)^2+(r_2)^2+(r_1)*(r_2))",
        "pi*(r_1+r_2)*a",
        "pi*a*(r_1+r_2) + pi*((r_1)^2+(r_2)^2)",
    ],
    ["bol", "4/3*pi*r^3", "4*pi*r^2"],
    ["bolzone", "pi*(r_1)^2*h/2+pi*(r_2)^2*h/2+pi*h^3/6", "2*pi*r*h"],
    ["bolkap", "1/3*pi*h^2*(3*r-h)", "2*pi*r*h"],
    ["bolschil", "1/6*pi*h*k^2"],
    ["bolsector", "2/3*pi*r^2"],
    ["piramide", "1/3*G*h"],
    ["afgeknotte priamide", "1/3*h*(G+B+sqrt(B*G))"],
]


def Main():
    for x in range(len(list_shapes)):  # Prints the name of all contained shapes
        print(x, list_shapes[x][0])

    figur_num = int(input("Figuur= "))  # Select the number of the shape
    print_shape_name(figur_num)
    print_formulas(figur_num)

    # If the mother contains 1 formula, it directly starts executing it
    if (len(list_shapes[figur_num]) - 1) == 1:
        formula_num = 1
    else:
        formula_num = int(input("Formule= "))
        print("\n{}".format(list_shapes[figur_num][formula_num]))

    solver(figur_num, formula_num)


# Prints the shapes name centered
def print_shape_name(figur_num):
    shape_name = list_shapes[figur_num][0]  # Takes the shape name
    # Calculate the amount of spaces to center the name of the shape
    spaces = int(SWIDTH / 2 - len(shape_name) / 2) * " "
    print("\n{}{}".format(spaces, shape_name.upper()))


# Prints all the availible formulas of the shape
def print_formulas(figur_num):
    # Starts counting by skipping the name
    for x in range(len(list_shapes[figur_num][1:])):
        # Fixes the array order by adding +1 to x (Number 0 is the mother already)
        print("{}. {}".format(x + 1, list_shapes[figur_num][x + 1]))


def solver(figur_num, formula_num):
    # "^" for powers in math means something else in Python, thereby we need to change all of them to "**" which is the right operator for this
    formula = list_shapes[figur_num][formula_num].replace("^", "**")

    while True:
        try:
            print("Antwoord: {}".format(eval(formula)))
            break  # Will break the function if the line above doesn't outputs error
        except Exception as error:
            var_error = str(error)

            index2 = var_error.find("' ") # The name of the variables is always followed by "' "

            # Index 1 is always 6 in the error string
            variable = var_error[6:index2] + "=" + input(var_error[6:index2] + "= ")
            exec(variable)


Main()
\$\endgroup\$
2
  • \$\begingroup\$ The code seems incomplete. SWIDTH is not defined. Also, what version of Python are you targeting? \$\endgroup\$
    – AJNeufeld
    Commented May 19, 2021 at 0:43
  • \$\begingroup\$ Indeed, I thought that I never used it and removed it. Now I added it back tho, thanks. \$\endgroup\$ Commented May 19, 2021 at 18:43

2 Answers 2

2
\$\begingroup\$

Data structures

You’re using the wrong data structure to contain your shapes and formulas. list_shapes is a list of lists, but the first item of every list is different from the other items in the list. Different kinds of things should be stored in the same container.

A simple change is to just store the formulas in another sub list:

list_shapes = [
    ["cilinder", ["pi*r^2*h", "2*r*h", "2*r*h + 2*pi*r^2"]],
    ["kegel", ["1/3*pi*r^2*h", "pi*r*a", "pi*r*a + pi*r^2"]],
    ...
    ]

Alternatively, you could store the data as a mapping.

list_shapes = {
    "cilinder": ["pi*r^2*h", "2*r*h", "2*r*h + 2*pi*r^2"],
    "kegel": ["1/3*pi*r^2*h", "pi*r*a", "pi*r*a + pi*r^2"],
    ...
    }

Best would be inside your own user-defined class.

Naming

PEP 8 states that variables names, function names, and parameter names should be in snake_case, with lowercase letters only. Capital letters are reserved use with constants or class names.

With this in mind, Main should be named main.

Loop like a Native

(See Ned’s Loop like a Native talk.)

The loop structure for x in range(len(list_shapes)): is frowned upon in polite Python circles. It is considered better to loop directly over the list itself, rather than looping over a list of list indices.

Why is it frowned upon? It is slower, less efficient. As an extreme example, consider a linked list. To determine the length of the list, one would need to traverse the entire list counting the items. Then, to get the n-th item, you would start from the head, and move forward n times. With the original loop, this becomes looping from the head to the 0th item, then looping from the head to the first item, then looping from the head to the second item, and so on, yielding \$O(n^2)\$ performance. It is more efficient to maintain an interator which advances over the list items one at a time.

For instance, to print out the shape names (using your original list_shapes structure), one could simply write:

for shape_data in list_shapes:
    print(shape_data[0])

In your case, you want the indices as well as the shape data. To get that, we use enumerate():

for x, shape_data in enumerate(list_shapes):
    print(x, shape_data[0])

This is more efficient than the original loop, since list_shape[x] never needs to be indexed.

We can improve this further using list unpacking. Using my first alternate list_shape definition, shape_data would be a list of two items. The shape name and a formula list, instead of assigning that data to shape_data, lets assign it to two variables: shape_name and formula_list:

for x, (shape_name, formula_list) in enumerate(list_shapes):
    print(x, shape_name)

Since we are not using the formula_list at this point, we could instead assign to the throw-away variable, _:

for x, (shape_name, _) in enumerate(list_shapes):
    print(x, shape_name)

Much clearer.

Centering

You go through a lot of effort to print out the uppercase shape name centered. This functionality is built-in to Python’s .format() function.

Centred in a 40 character-wide field:

print("{:^40}".format(shape_name.upper())

Or using Python 3.6’s f-strings:

print(f"{shape_name.upper():^40}")

When SWIDTH is defined, you can use it too:

print(f"{shape_name.upper():^{SWIDTH}}")

Loop like a Native (reprise)

This is wrong on so many levels:

    for x in range(len(list_shapes[figur_num][1:])):
        print("{}. {}".format(x + 1, list_shapes[figur_num][x + 1]))

First, you take list_shapes[figur_num] and this slice it with [1:], creating a new list in memory, from which you just compute the len(...)! You could have computed the length of the original and subtracted one, saving the Python interpreter a lot of time and memory.

Next, we have the for x in range(len(…)) structure we already mentioned.

Then, we compute x + 1 twice per loop iteration!

Lastly, insult to injury, we are looking up list_shapes[figur_num] on each iteration, plus an additional time to compute the length!

Let’s rework this:

    for index, formula in enumerate(list_shapes[figur_num][1:], 1):
        print("{}. {}".format(index, formula))

We’ve still got the list slice, but doesn’t seem as awful here, since we are actually iterate over the items of the slice. list_shapes[figur_num] is only looked up once. And the + 1 calculation vanished from the loop entirely, because we use enumerate(..., 1) to start the enumeration value at 1.

With the improved list_shapes structure, where the shape name and formulas are separate, we can eliminate the slice, too:

    for index, formula in enumerate(list_shapes[figur_num][1]):
        print("{}. {}".format(index, formula))

Exception Handling

When eval(formula) raises an exception, you extract the variable name which was not defined.

What if the exception was a ZeroDivisionError, or maybe ValueError due to square-root of a negative number?

You should catch the more precise exception you can to avoid catching and attempting to recover from the wrong problem.

    try:
        print(eval(formula))
    except NameError as error:
        ...

Again, you’re going through a lot of work to extract the variable name from the error. It could be simpler:

        variable_name = str(error).split("'")[1]

Local variables

When you use exec(…) or eval(…), your global and local environment are available to use and modify. This include variables like formula and formula_num and functions like Main.

You should use a clean environment for your evaluator.


   local_vars = {}
   …
   while True:
       eval(formula, None, local_vars)
       …
       exec(variable, None, local_vars)

Initially, local_vars won’t contain any variables. But as the exec(…, None, local_vars) statement gets executed, your user-defined variables will start to be added to local_vars, and used in the eval(…, None, local_vars) statement.

This means to run a different calculation, possibly with a different formula, you can re-initalize local_vars to an empty dictionary, and you will be able to enter new values for the variables.

\$\endgroup\$
2
  • 1
    \$\begingroup\$ i'd also add about the import * \$\endgroup\$
    – hjpotter92
    Commented May 19, 2021 at 2:38
  • \$\begingroup\$ Thanks a lot for the effort, you gave me a pretty good insight in improving the code and how I should think while coding. \$\endgroup\$ Commented May 19, 2021 at 10:52
2
\$\begingroup\$

formulas

It looks like some formulas are incorrect. For example, some formulas for cilinder look like they are missing a pi term.

separation of concerns

A function or method should only need to know what is necessary for it to do it's job. print_shape_name, print_formulas, and solver all take figur_num as parameters, solver also takes formula_num. So each function needs to know the structure of list_shapes. If you decide to change list_shapes to a different data structure, such as a dict, you need to change all the functions. Instead, recode the functions to accept the shape name, formula, or formulas.

For example:

def print_shape_name(shape_name):
    print(f"\n{shape_name:^30}")


def print_formulas(formulas):
    for n, formula enumerate(formulas, 1):
        print(f"{n}. {formula}")

getting formula variables

Catching Exceptions from eval to prompt the user for values used in the formula seems very "hackish". It is fairly simple to scan the formulas for variables and ask the user for values.

import re

def get_formula_variables(formula):
    variables = set(re.findall(r'[a-zA-Z][_a-zA-Z0-9]*', formula))
   
    formula_vars = {'pi':pi}
    
    for variable in variables:
        if variable not in formula_vars:
            value = input(f"{variable} = ")
            formula_vars[variable] = float(value)

    return formula_vars

Then solve would look like:

def solver(formula):
    formula = formula.replace('^', '**')
    
    formula_vars = get_formula_variables(formula)
            
    return eval(formula, None, formula_vars)

user experience

It would get rather tedious to reenter the parameters when you needed more than one equation for a shape. Revise the caclulator to ask for the shapes parameters when the shape is selected. Then when a formula is selected the parameters are already known until a new shape is selected. It could just print all the formulas for the shape.

def main():
    for n, shape in enumerate(list_shapes):
        print(n, shape[0])

    figur_num = int(input("Figuur= "))

    shape_name, *formulas = list_shapes[figur_num]
    print_shape_name(shape_name)

    formula_vars = get_formula_variables(' '.join(formulas))

    for formula in formulas:
        formula = formula.replace('^', '**')
        result = eval(formula, None, formula_vars)
        print(f"{formula} = {result}")

Note - I haven't tested the code much so there may be some bugs or typos, but is should give you some ideas to work with.

\$\endgroup\$
3
  • \$\begingroup\$ The problem with import re is that I cannot do that action on my calculator, it is runs micropython and doesn't have regex. \$\endgroup\$ Commented May 19, 2021 at 18:37
  • \$\begingroup\$ @Betraxa, it would be easy enough to add a field to list_shapes for the shape's parameters: list_shapes = [["cilinder", "r h", ["pi*r^2*h", "2*r*h", "2*r*h + 2*pi*r^2"]], ...]. Then use .split() on that field instead of the regex. \$\endgroup\$
    – RootTwo
    Commented May 19, 2021 at 19:04
  • \$\begingroup\$ That was my first thought as well, however my priority is to make the writing part as simple as possible (I know it is not that much work, but it is a personal choice). I am writing this script to make my life easier at the end :) \$\endgroup\$ Commented May 19, 2021 at 20:07

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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