Overall this seems pretty good. The calculations look correct (given sensible input) and the code is easy to follow.
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.