7
\$\begingroup\$

I created an area of shapes calculator. It's my first python script and I just wanted to get a few tips, thanks in advance!

#Area of selected polygons and polyhedrons calculator.
import sys
import math
on=0
print("This is an area (or surface area) calculator for selected 2D and 3D shapes, press 0 to start.")
start=input()
if start=="0":
    on=1
    print("Input 1 to navigate to 2D shapes, 2 to navigate to 3D shapes and q to terminate the program.")   
elif start=="q":
        sys.exit() 
else:   
    print("Error! Command invalid, restart required.")
while on==1:
    oink=input()
    if oink=="1":
        print("Enter 2 for any Oval/Circle, 3 for any Triangle, 4 for any convex Quadrilateral, 5 for regular Pentagon, 6 for regular Hexagon and 7 for any other regular 2D polygon.")
        spoink=input()
        if spoink=="2":
            print("You have selected, Oval/Circle, input length of major axis (axes will be equal for a circle.)")
            majoraxis=float(input())
            print("Now, input length of minor axis.")
            minoraxis=float(input())
            print("The area of your Oval/Circle is " + str(math.pi*(minoraxis/2)*(majoraxis/2))+("."))
        elif spoink=="3":
            print("You have selected, Triangle, input height.")
            TRIHeight=float(input())
            print("Input base")
            TRIBase=float(input())
            print("The area of your Triangle is " + str((TRIBase*TRIHeight)/2)+("."))
        elif spoink=="4":
            print("You have selected, Quadrilateral, input length of each of the four sides in A-B-C-D order.")
            QUA1=float(input())
            QUA2=float(input())
            QUA3=float(input())
            QUA4=float(input())
            print("Now, input 2 opposite angles.")
            QUAAng1=float(input())
            QUAAng2=float(input())
            s=((QUA1+QUA2+QUA3+QUA4)/2)
            print("The area of your Quadrilateral is " + str((math.sqrt(((s-QUA1)*(s-QUA2)*(s-QUA3)*(s-QUA4))-(((math.cos(QUA1*QUA2*QUA3*QUA4))*(math.cos(QUA1*QUA2*QUA3*QUA4)))*((QUAAng1+QUAAng2)/2)))))+("."))
        elif spoink=="5":
            print("You have selected, Pentagon, input side length.")
            PenSidelength=float(input())
            print("The area of your Pentagon is " + str(0.25*(math.sqrt(5*(5+(2*math.sqrt(5))))*(PenSidelength**2))))  
        elif spoink=="6":
            print("You have selected, Hexagon, input side length.")
            HexSidelength=float(input())
            print("The area of your Hexagon is " + str(((3*math.sqrt(3))*(HexSidelength**2))/2))
        elif spoink=="7":
            print("You have selected, regular Polygon, input numer of sides.")
            Poln=float(input())
            print("Now, input side length")
            Pols=float(input())
            print("The area of your regular Polygon is " + str(Poln*(Pols**2)*(1/math.tan(math.pi/Poln))/4)+("."))
        elif spoink=="q":
                sys.exit()
        else:
            print("Error! Input invalid, reenter.")
    elif oink=="2": 
        print("Enter 8 for Sphere, 9 for Tetrahedron, 10 for Cube/Cuboid, 11 for Octahedron, 12 for Dodecahedron and 13 for Icosahedron.")
        boink=input()
        if boink=="8":
            print("You have selected, Sphere, input radius.")
            radius=float(input())
            print("The surface area of your Sphere is "+str(4*math.pi*(radius**2))+("."))
        elif boink=="9":
            print("You have selected, Tetrahedron, input the edge length.")
            TETlen=float(input())
            print("The surface area of your Tetrahedron is "+str((math.sqrt(3))*(TETlen**2))+("."))
        elif boink=="10":
            print("You have selected, Cube/Cuboid, input the height.")
            CUBhei=float(input())
            print("Now, input the width")
            CUBwid=float(input())
            print("Finally, input the depth")
            CUBdep=float(input())
            print("The surface area of your Cube/Cuboid is "+str(2*(CUBhei*CUBwid+CUBhei*CUBdep+CUBwid*CUBdep))+("."))
        elif boink=="11":
            print("You have selected Octahedron, input side length.")
            OCTlen=float(input())
            print("The surface area of your Octahedron is "+str((2*(OCTlen**2))*(math.sqrt(3)))+("."))
        elif boink=="12":
            print("You have selected Dodecahedron, input side length.")
            DOClen=float(input())
            print("The surface area of your Dodecahedron is "+str(3*(DOClen**2)*(math.sqrt(5*(5+(2*math.sqrt(5))))))+("."))
        elif boink=="13":
            print("You have selected Icosahedron, input side length.")
            ICOlen=float(input())
            print("The surface area of your Icosahedron is "+str(5*(ICOlen**2)*(math.sqrt(3)))+("."))
        elif boink=="q":
            sys.exit()
        else:
            print("Error! Input invalid, reenter.")
    elif oink=="q":
        sys.exit()
    else:
     print("Error! Input invalid, reenter.")
\$\endgroup\$
7
\$\begingroup\$
  • Whew, first, let's breathe.

Don't you think everything's a bit congested? Let's put in a few newlines in between and add spaces between arithmetic operators.

You have repeated float(input()) exactly 22 times! How about we write a function that takes the input for you?

def float_input():
    return float(input())

Now, you can call this function instead of using float(input())!

  • How about defining some constants next? Even though you can't declare constants in python, we can treat it as such!

For example, we can define PI = math.pi and using it instead of math.pi? The code would look shorter and sweeter.

  • Next step is to use formatting!

"The area of your Triangle is {}" + str((TRIBase * TRIHeight) / 2)+(".")

Could be changed to

f"The area of your Triangle is {(TRIBase * TRIHeight) / 2}."

  • Write functions instead of lengthy formulas to make your code neater

Here's the final code after applying all the points above and making a few more negligible changes!

# Imports and constants
import sys
from math import sqrt, pi

PI = pi

float_input = lambda: float(input())


# 2d functions
def area_of_triangle(height, base):
    return height * base / 2


def area_of_oval(majoraxis, minoraxis):
    return PI * (minoraxis / 2) * (majoraxis / 2)


def area_of_quadrilateral(side_length_list, ang1, ang2):
    s = sum(side_length_list)
    prod = mult = 1

    for i in side_length_list:
        prod *= i
        mult *= s - i

    return sqrt(mult - (pow(math.cos(prod), 2) * ((ang1 + ang2) / 2)))


def area_of_pentagon(side_length):
    return 0.25 * sqrt(5 * (5 + (2 * sqrt(5)))) * pow(side_length, 2)


def area_of_hexagon(side_length):
    return (3 * sqrt(3)) * (pow(side_length, 2)) / 2


def area_of_polygon(n, s):
    return n * pow(s, 2) * (1 / math.tan(PI / n)) / 4


# 3d functions
def area_of_sphere(radius):
    return 4 * PI * (radius ** 2)


def area_of_tetrahedron(side_length):
    return pow(side_length, 2) * sqrt(3)


def area_of_cuboid(height, breadth, length):
    return 2 * (height * breadth + height * length + length * breadth)


def area_of_octahedron(side_length):
    return 2 * pow(side_length, 2) * sqrt(3)


def area_of_dodecahedron(side_length):
    return 3 * pow(side_length, 2) * sqrt(5 * (5 + 2 * sqrt(5)))


def area_of_icosahedron(side_length):
    return 5 * pow(side_length, 2) * sqrt(3)


# The main function
def main():
    while True:
        print("This is an area (or surface area) calculator for selected 2D and 3D shapes, press 0 to start.")
        start = input()

        if start == "0":
            print("Input 1 to navigate to 2D shapes, 2 to navigate to 3D shapes and q to terminate the program.")

        elif start == "q":
            sys.exit()

        else:
            print("Error! Command invalid.")
            continue

        while True:
            oink = input()

            if oink == "1":
                print(
                    "Enter 2 for any Oval/Circle, 3 for any Triangle, 4 for any convex Quadrilateral, 5 for regular Pentagon, 6 for regular Hexagon and 7 for any other regular 2D polygon.")
                spoink = input()

                if spoink == "2":
                    print(
                        "You have selected, Oval/Circle, input length of major axis (axes will be equal for a circle.)")
                    majoraxis = float_input()

                    print("Now, input length of minor axis.")
                    minoraxis = float_input()

                    print(f"The area of your Oval/Circle is {area_of_oval(majoraxis, minoraxis)}")

                elif spoink == "3":
                    print("You have selected, Triangle, input height.")
                    height = float_input()

                    print("Input base")
                    base = float_input()

                    print(f"The area of your Triangle is {area_of_triangle(height, base)}.")

                elif spoink == "4":
                    print("You have selected quadrilateral, input length of each of the four sides in A-B-C-D order.")
                    side_length_list = [float_input() for _ in range(4)]

                    print("Now, input 2 opposite angles.")

                    ang1 = float_input()
                    ang2 = float_input()

                    print(f"The area of your Quadrilateral is {area_of_quadrilateral(side_length_list, ang1, ang2)}")

                elif spoink == "5":
                    print("You have selected, Pentagon, input side length.")
                    side_length = float_input()
                    print(f"The area of your Pentagon is {area_of_pentagon(side_length)}")

                elif spoink == "6":
                    print("You have selected, Hexagon, input side length.")
                    side_length = float_input()
                    print(f"The area of your Hexagon is {area_of_hexagon(side_length)}")

                elif spoink == "7":
                    print("You have selected, regular Polygon, input numer of sides.")
                    n = float_input()

                    print("Now, input side length")
                    s = float_input()

                    print(f"The area of your regular Polygon is {area_of_polygon(n, s)}.")

                elif spoink == "q":
                    sys.exit()

                else:
                    print("Error! Input invalid, reenter.")

            elif oink == "2":
                print(
                    "Enter 8 for Sphere, 9 for Tetrahedron, 10 for Cube/Cuboid, 11 for Octahedron, 12 for Dodecahedron and 13 for Icosahedron.")
                boink = input()

                if boink == "8":
                    print("You have selected, Sphere, input radius.")
                    radius = float_input()
                    print(f"The surface area of your Sphere is {area_of_sphere(radius)}.")

                elif boink == "9":
                    print("You have selected, Tetrahedron, input the edge length.")
                    side_length = float_input()
                    print(f"The surface area of your Tetrahedron is {area_of_tetrahedron(side_length)}.")

                elif boink == "10":
                    print("You have selected, Cube/Cuboid, input the height.")
                    height = float_input()

                    print("Now, input the breadth")
                    breadth = float_input()

                    print("Finally, input the length")
                    length = float_input()

                    print(f"The surface area of your Cube/Cuboid is {area_of_cuboid(height, breadth, length)}.")

                elif boink == "11":
                    print("You have selected Octahedron, input side length.")
                    side_length = float_input()
                    print(f"The surface area of your Octahedron is {area_of_octahedron(side_length)}.")

                elif boink == "12":
                    print("You have selected Dodecahedron, input side length.")
                    side_length = float_input()
                    print(f"The surface area of your Dodecahedron is {area_of_dodecahedron(side_length)}")

                elif boink == "13":
                    print("You have selected Icosahedron, input side length.")
                    side_length = float_input()
                    print(f"The surface area of your Icosahedron is {area_of_icosahedron(side_length)}")

                elif boink == "q":
                    sys.exit()

                else:
                    print("Error! Input invalid, reenter.")

            elif oink == "q":
                sys.exit()

            else:
                print("Error! Input invalid, re-enter.")

EDIT: I kept the names onik, spoink, and boink as it is, because I think the pronunciation is so cute!

\$\endgroup\$
  • 1
    \$\begingroup\$ You should use from math import sqrt instead of using an import and an assignment. Use from math import pi as PI to import and rename. \$\endgroup\$ – AJNeufeld Nov 9 at 14:43
  • \$\begingroup\$ @AJNeufeld Yes, I agree. I've edited my answer as such. \$\endgroup\$ – Srivaths Nov 9 at 19:10
  • \$\begingroup\$ Thanks for the help, alot of this stuff I didn't know existed. If I may ask what is the point of putting everything in a function? \$\endgroup\$ – Sp1Te Nov 10 at 15:48
  • \$\begingroup\$ You don't have to repeat the code if you have functions. You could write a function for a formula a and call it for all instances you use that formula. For example, you could have a function area_of_rectangle and use height * area_of_rectangle(length, breadth) to get a cuboid. Also, functions make the code intact without formulas laying everywhere around. \$\endgroup\$ – Srivaths Nov 11 at 10:36
6
\$\begingroup\$

Tool support suggestions:

  1. black can automatically format your code to be more idiomatic.
  2. isort can group and sort your imports.
  3. flake8 with a strict complexity limit can give you more hints to write idiomatic Python:

    [flake8]
    max-complexity = 4
    ignore = W503,E203
    
  4. I would then recommend validating your type hints using a strict mypy configuration:

    [mypy]
    check_untyped_defs = true
    disallow_any_generics = true
    disallow_untyped_defs = true
    ignore_missing_imports = true
    no_implicit_optional = true
    warn_redundant_casts = true
    warn_return_any = true
    warn_unused_ignores = true
    

    This ensures that anyone reading the code (including yourself) understand how it's meant to be called, which is very powerful in terms of modifying and reusing it.

Basically the above gives some basic quality assurances which come with several advantages:

  • Makes the code easier to read for anyone familiar with idiomatic Python.
  • Smaller diffs which are easier to review and understand.
  • Less risk of misunderstanding semantics.

Specific suggestions, assuming the above are taken care of:

  1. The variable names need some work. on, oink and spoink give a reader absolutely no idea what the variables are used for. spoink, for example, could be shape or even shape_name.
  2. The various calculations belong in separate functions. The 2D and 3D functions could also be split into two separate files, for clarity.
  3. A common expectation for shell tools is that they are not interactive unless they absolutely have to be. So shell users would expect your script to take all the parameters it needs (using for example argparse) to produce an answer non-interactively. I might for example write a script like this to run like ./area.py circle 5 to calculate the area of a circle of radius 5.
\$\endgroup\$
  • \$\begingroup\$ +1 for variable names rework! \$\endgroup\$ – Abdur-Rahmaan Janhangeer Nov 9 at 11:32
  • \$\begingroup\$ I agree with you on all points, but don't you think it's a lot to digest for a beginner? The OP said It's my first python script \$\endgroup\$ – Srivaths Nov 10 at 5:39
  • 1
    \$\begingroup\$ My approach is basically to give users as much actionable suggestions as I can. It's up to them how deep they want to dig into that. Basically this is the kind of code review I would have wanted to receive at an early stage, to prepare for the reality of production code. \$\endgroup\$ – l0b0 Nov 10 at 6:06
  • \$\begingroup\$ Thanks for the help, to clarify for your last point do you mean that I write so the user inputs preset commands with little to no instruction? Without any of the navigation or choice. \$\endgroup\$ – Sp1Te Nov 10 at 15:52
  • \$\begingroup\$ Basically, with argparse you can set up which arguments the script takes, and it'll add a --help option by default which shows what you have configured. So users can still get instruction on how to use it. \$\endgroup\$ – l0b0 Nov 10 at 18:56

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.