4
\$\begingroup\$

I'm pretty new with programming in general. I'm using Python 2 right now as it seems most of the courses are teaching Python 2 and not 3. I built this area calculator that will calculate square, rectangle, triangle, circle or trapezoid. I'm a little concerned about how I used the functions as well as my else statement when an "invalid" input is given.

import math

##formulas for each geometric figure

def calc_square(a_side):
    square_area = a_side ** 2
    return square_area

def calc_rectangle(w_side, l_side):
    rect_area = l_side * w_side
    return rect_area

def calc_triangle(base, height):
    triangle_area = (base * height) / 2
    return triangle_area

def calc_circle(radius):
    circle_area = math.pi * radius ** 2
    return circle_area

def calc_trapezoid(short_base, long_base, height):
    trapezoid_area = ((short_base + long_base) / 2 ) * height
    return trapezoid_area

##function determining which formula to calculate

def area_calc_logic(user_calc):
    if user_calc == "square":
        a_side = float(raw_input("Give length of side: "))
        print calc_square(a_side)
    elif user_calc == "rectangle":
        l_side = float(raw_input("Give the length: "))
        w_side = float(raw_input("Give the width: "))
        print calc_rectangle(w_side, l_side)
    elif user_calc == "triangle":
        base = float(raw_input("Give the length of base: "))
        height = float(raw_input("Give the height: "))
        print calc_triangle(base, height)
    elif user_calc == "circle":
        radius = float(raw_input("Give the radius: "))
        print calc_circle(radius)
    elif user_calc == "trapezoid":
        short_base = float(raw_input("Give the length of the short base: "))
        long_base = float(raw_input("Give the length of the long base: "))
        height = float(raw_input("Give the height: "))
        print calc_trapezoid(short_base, long_base, height)
    else:
        area_calc_logic(raw_input("Error, Re-enter input: "))


print "This program will calculate/narea of some geometric shapes for you"
print "Shapes available are squares, triangles, circles, and trapezoid"
print "Enter square, rectangle, triangle, circle, or trapezoid"
area_calc_logic(raw_input("What area would you like to calculate? "))
\$\endgroup\$
3
\$\begingroup\$

Overall this seems pretty good. The calculations look correct (given sensible input) and the code is easy to follow.

Here are some suggestions for making this even better:

  • Cope with the user entering an empty string.

    If I select a shape, and then press “enter” immediately when asked for a dimension, I get a nasty traceback printed to the console:

    This program will calculate/narea of some geometric shapes for you
    Shapes available are squares, triangles, circles, and trapezoid
    Enter square, rectangle, triangle, circle, or trapezoid
    What area would you like to calculate? square
    Give length of side:
    Traceback (most recent call last):
      File "areas.py", line 54, in <module>
        area_calc_logic(raw_input("What area would you like to calculate? "))
      File "areas.py", line 29, in area_calc_logic
        a_side = float(raw_input("Give length of side: "))
    ValueError: could not convert string to float:
    

    If you add a bit of debugging code around that, you see it’s trying to process float('') and choking. You should probably ask the user for the dimension again.

  • Handle negative dimensions.

    For example, if I tell you my triangle has base -5 and height 14, you tell me that the area is -35. That seems a bit odd – isn’t area always positive?

    It’s not immediately obvious how you should handle negative quantities from the user – perhaps just call abs() on their input? – but the current approach isn’t very good.

  • Move the area_user_logic into an if __name__ == '__main__' block.

    This just means tweaking the end of the program as follows:

    if __name__ == '__main__':
        print "This program will calculate/narea of some geometric shapes for you"
        print "Shapes available are squares, triangles, circles, and trapezoid"
        print "Enter square, rectangle, triangle, circle, or trapezoid"
        area_calc_logic(raw_input("What area would you like to calculate? "))
    

    This means that the interactive session only runs when you call the script directly. If I import the file into another file, I don’t get the interactive session, just the functions you’ve defined.

    This helps make your code more reusable. See What does if __name__ == '__main' do? on SO for more details.

  • Cope with weirdly cased input.

    I claim my intention is perfectly clear here, but your program rejects it:

    This program will calculate/narea of some geometric shapes for you
    Shapes available are squares, triangles, circles, and trapezoid
    Enter square, rectangle, triangle, circle, or trapezoid
    What area would you like to calculate? Triangle
    Error, Re-enter input:

    It would be good to lowercase user input before checking – that makes it a bit more friendly to malformed user input.

  • Print an integer area if possible.

    Here’s a sample run from your program:

    This program will calculate/narea of some geometric shapes for you
    Shapes available are squares, triangles, circles, and trapezoid
    Enter square, rectangle, triangle, circle, or trapezoid
    What area would you like to calculate? square
    Give length of side: 3
    9.0

    From my perspective, I’ve entered an integer, and numerically the area is an integer. It’s a bit odd that you returned it as a float. You could use the float.is_integer method to detect if an area has integral value, and print a slightly cleaner output.

  • The else branch could cause a recursion bug.

    If somebody enters bogus input 999 times, they’ll hit Python’s max recursion depth. Admittedly they’d need to be quite patient – I tried it, and got bored holding down “Enter” – but hey, would be nice to avoid.

    You could refactor the code into a while True: loop. Something like (pseudo-code):

    while True:
        ask_for_shape_name()
        if shape_name_is_valid():
            ask_for_dimensions()
            print_area_given_shape_and_dimensions()
        else:
            error_message()
    

    as a bonus, this approach could allow me to calculate many areas at once, rather than restarting the script every time.

  • Python 3 support isn’t tricky for this script.

    There are only two changes that affect this code:

    • print is a function, not a statement. That means you write print("hello world") instead of print "hello world". If you want to use this in Python 2 – which will help you write scripts than run under 2 or 3 – add this line to the top of your script:

      from __future__ import print_function
      

      It forces your Python 2 scripts to use print() as a function.

    • In Python 2, there’s a distinction between raw_input(), which returns user input as a string, and input(), which eval()'s the input first.

      The latter is quite dangerous, so in Python 3 raw_input() was renamed to input(), and the old input() was dropped. To get the old behaviour, you have to do eval(input()), which makes the danger more explicit.

| improve this answer | |
\$\endgroup\$
0
\$\begingroup\$

Removing duplication

In each if branch, you ask some floats and then apply a formula to them. Only the number of floats, the messages printed and the formula change, the basic structure remains the same.

I suggest using this function to abstract the asking floats away:

def ask_floats(how_many, prompts):
    return [float(raw_input(p)) for p in range(how_many)]

Usage is like:

elif user_calc == "rectangle":
    print calc_rectangle(* ask_floats(2, ["The length? ", "The width? "]))
elif user_calc == "triangle":
    print calc_triangle(* ask_floats(2, ["The base? ", "The height? "]))
| improve this answer | |
\$\endgroup\$

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