debugpythonMinor
Simple MPG calculator in Python
Viewed 0 times
pythonsimplecalculatormpg
Problem
I am a self taught coder taking a Programming Fundamentals class to work towards a degree. It's based on Python, which I'm not as familiar with as other languages. I added error handling like I would in javascript or php to be an overachiever. Professor gave online feedback (online course) not to use try-except, that it is ahead of where we are in the semester, that even when we do get there, she doesn't want it in our code, and that this code is "not well formed" because I used try-except. She gave no feedback as to what she meant and I don't think that sounds right but don't now enough to stand my ground. Googling "well formed python" just give me a bunch of xml.
```
# main function
def main():
# @input dist : distance traveled in miles
# @input guse : gas used traveling dist
# @output mpg : dist/guse
# set dist from input "Enter miles traveled: "
dist = input('Enter distance traveled(in miles): ')
# make sure dist is a number, and if not print error and restart
try:
dist = float(dist)
except ValueError:
print('Distance traveled must be a number! You did not travel "' + \
dist + '" miles!')
main()
else:
# if dist is 0.0, print error and restart
try:
x = 1 / dist
except ZeroDivisionError:
print('Miles traveled cannot be 0!')
main()
else:
# set guse from input "Enter gas used in gallons: "
gcon = input('Enter gas consumed traveling(in gallons): ')
# make sure guse is a number, and if not print error and restart
try:
gcon = float(gcon)
except ValueError:
print('Gas consumed must be a number. You did not use "' + \
gcon + '"" gallons of gas!')
main()
else:
# set mpg to dist divided by guse
# if guse is 0.0, print error and restart
```
# main function
def main():
# @input dist : distance traveled in miles
# @input guse : gas used traveling dist
# @output mpg : dist/guse
# set dist from input "Enter miles traveled: "
dist = input('Enter distance traveled(in miles): ')
# make sure dist is a number, and if not print error and restart
try:
dist = float(dist)
except ValueError:
print('Distance traveled must be a number! You did not travel "' + \
dist + '" miles!')
main()
else:
# if dist is 0.0, print error and restart
try:
x = 1 / dist
except ZeroDivisionError:
print('Miles traveled cannot be 0!')
main()
else:
# set guse from input "Enter gas used in gallons: "
gcon = input('Enter gas consumed traveling(in gallons): ')
# make sure guse is a number, and if not print error and restart
try:
gcon = float(gcon)
except ValueError:
print('Gas consumed must be a number. You did not use "' + \
gcon + '"" gallons of gas!')
main()
else:
# set mpg to dist divided by guse
# if guse is 0.0, print error and restart
Solution
I have three main remarks:
-
Recursion is not really an appropriate retry mechanism.
Try this: hold down Enter for a dozen responses, then hit ControlC. As you see, the stack trace is quite deep. It would be better for the call stack to be the same regardless of how the user arrived at a certain point in the code, because invariants make software easier to test and debug. You're using recursion as a GOTO, except it's worse because it leaves a trace. Instead, the validation should be done in a loop, which you break out of when valid input is obtained.
-
The code structure is a bit repetitive.
The routines to input the distance and gas consumption are actually quite similar, and could be written using shared code.
-
Trial division is a poor way to test for a zero value.
You test whether
There are more straightforward ways to test for a zero value! You end up discarding
On further thought, your validation is not quite right. If you're going to validate your input, you might as well prohibit negative numbers. It is possible for the distance to be zero, when the car has been idling. (The zero-distance case would only be problematic when displaying efficiency in "metric" units of L / 100km.)
In conclusion, your use of exceptions for validation is partly inappropriate (catching
A minor note: your comments are inconsistent with your code. Your code has a variable named
Suggested solution:
-
Recursion is not really an appropriate retry mechanism.
Try this: hold down Enter for a dozen responses, then hit ControlC. As you see, the stack trace is quite deep. It would be better for the call stack to be the same regardless of how the user arrived at a certain point in the code, because invariants make software easier to test and debug. You're using recursion as a GOTO, except it's worse because it leaves a trace. Instead, the validation should be done in a loop, which you break out of when valid input is obtained.
-
The code structure is a bit repetitive.
The routines to input the distance and gas consumption are actually quite similar, and could be written using shared code.
-
Trial division is a poor way to test for a zero value.
You test whether
dist is zero with# if dist is 0.0, print error and restart
try:
x = 1 / dist
except ZeroDivisionError:
print('Miles traveled cannot be 0!')
main()There are more straightforward ways to test for a zero value! You end up discarding
x, so the trial division isn't even useful work.On further thought, your validation is not quite right. If you're going to validate your input, you might as well prohibit negative numbers. It is possible for the distance to be zero, when the car has been idling. (The zero-distance case would only be problematic when displaying efficiency in "metric" units of L / 100km.)
In conclusion, your use of exceptions for validation is partly inappropriate (catching
ZeroDivisionError to test for zero, especially for dist, from a logical viewpoint as well as a code expression viewpoint) and partly appropriate (catching ValueError to test for strings that cannot be interpreted as numbers). I don't see any alternative to catching ValueError, except perhaps checking the input string against a regular expression \d+(?:\.(?:\d*))?, and I would consider ValueError to be the superior approach. I think it would be fair to challenge your professor to propose a better alternative — in my opinion, the ball is in her court.A minor note: your comments are inconsistent with your code. Your code has a variable named
gcon; the comments mention guse.Suggested solution:
def ask(prompt, typeconv, typeconv_err, validation=None, validation_err=None):
"""
Displays the prompt and asks for user input. The result is passed through
the typeconv function, then possibly a validation function as well. If
either the type conversion fails (with a ValueError) or the validation
fails (by being false value), then the respective error message is
displayed, and the prompt is displayed anew.
"""
while True:
val = input(prompt)
try:
val = typeconf(val)
except ValueError:
print(typeconv_err % (val))
else:
if validation is not None and not validation(val):
print(validation_err % (val))
else:
return val
def main():
dist = ask('Enter distance traveled(in miles): ',
float,
'Distance traveled must be a number! You did not travel "%s" miles!',
lambda dist: dist >= 0.0,
'Miles traveled cannot be negative!')
gcon = ask('Enter gas consumed traveling(in gallons): ',
float,
'Gas consumed must be a number. You did not use "%s" gallons of gas!',
lambda gcon: gcon > 0.0,
'Gas consumed cannot be 0 or negative!')
# print mpg
mpg = dist / gcon
print('Average miles per gallon(mpg):', format(mpg, '5.2f'))
# print request for enter keypress to end
input('press enter to continue')
# Call main function
main()Code Snippets
# if dist is 0.0, print error and restart
try:
x = 1 / dist
except ZeroDivisionError:
print('Miles traveled cannot be 0!')
main()def ask(prompt, typeconv, typeconv_err, validation=None, validation_err=None):
"""
Displays the prompt and asks for user input. The result is passed through
the typeconv function, then possibly a validation function as well. If
either the type conversion fails (with a ValueError) or the validation
fails (by being false value), then the respective error message is
displayed, and the prompt is displayed anew.
"""
while True:
val = input(prompt)
try:
val = typeconf(val)
except ValueError:
print(typeconv_err % (val))
else:
if validation is not None and not validation(val):
print(validation_err % (val))
else:
return val
def main():
dist = ask('Enter distance traveled(in miles): ',
float,
'Distance traveled must be a number! You did not travel "%s" miles!',
lambda dist: dist >= 0.0,
'Miles traveled cannot be negative!')
gcon = ask('Enter gas consumed traveling(in gallons): ',
float,
'Gas consumed must be a number. You did not use "%s" gallons of gas!',
lambda gcon: gcon > 0.0,
'Gas consumed cannot be 0 or negative!')
# print mpg
mpg = dist / gcon
print('Average miles per gallon(mpg):', format(mpg, '5.2f'))
# print request for enter keypress to end
input('press enter to continue')
# Call main function
main()Context
StackExchange Code Review Q#40596, answer score: 8
Revisions (0)
No revisions yet.