I wrote a module that calculates the area of shapes and I wanted to get feedback. Am I on the right track?
import math
# A python program to calculate the area of shapes
# area of a square is length multiplied by width
def area_of_square(length, width):
_check_if_values_are_strings(length = length, width = width)
_check_if_values_are_negitive(lenth = length, width = width)
return length * width
# area of a triangle is base multiplied by height divided by 2
def area_of_triangle(base_value, height):
_check_if_values_are_strings(base_value = base_value, height = height)
_check_if_values_are_negitive(base_value = base_value, height = height)
return (base_value * height) / 2
# area of a circle is radius squared times pi
def area_of_circle(radius):
_check_if_values_are_strings(radius = radius)
_check_if_values_are_negitive(radius = radius)
return math.pow(radius,2) * math.pi
def _check_if_values_are_strings(**values):
for k,v in values.items():
if (isinstance(v,str)):
raise TypeError("{0} is a string".format(k))
def _check_if_values_are_negitive(**values):
for k,v in values.items():
if v < 0:
raise ValueError("{0} is a negitive".format(k))
My main concern is the use of the _check_if_values_are_strings
and _check_if_values_are_negitive
methods. Am I using them correctly, or is there a better approach to verifying the parameters before preforming the calculation?
2 Answers 2
Instead of having multiple _check_if_values_are_
somethings, just have a single check for the type of values you want. You want the values to be positive real number, just assert them:
def ensure_positive_reals(*values):
assert all(val >= 0 for val in values)
which will raise ValueError
if any of the val
was a string and an AssertionError
if any of them is negative (Thanks Mathias). To use it, you have:
ensure_positive_reals(radius)
ensure_positive_reals(length, width)
ensure_positive_reals(base_value, height)
Instead of math.pow
, use the exponentiation expression **
:
math.pi * (radius ** 2)
-
1\$\begingroup\$ Except
float(val)
will not error out on a string like"42"
but thenarea_of_square("42", 2)
will return'4242'
instead of the expected84
... Why not just useassert all(val >= 0 for val in values)
instead and let Python tell you aboutTypeError
s if anyval
is not a number? \$\endgroup\$301_Moved_Permanently– 301_Moved_Permanently2018年03月06日 09:42:11 +00:00Commented Mar 6, 2018 at 9:42 -
\$\begingroup\$ @MathiasEttinger ah yes. I didn't notice that. Thanks :) \$\endgroup\$hjpotter92– hjpotter922018年03月06日 09:49:22 +00:00Commented Mar 6, 2018 at 9:49
A few things:
- Squares don't have width and height, those are rectangles
- You check to see that your values are not
string
s, what if they arelist
s (or other types of objects)? - Your check functions error if their title is true, this is unintuitive. Something like
ensure_value_is_positive
would be a better name.
lenth = length
.) Not using any keyword in "the checks", why are you not using a single asterisk (args instead of kwargs)? \$\endgroup\$