7
\$\begingroup\$

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
                try:
                    mpg = dist / gcon
                except ZeroDivisionError:
                    print('Gas consumed can not be 0!')
                    main()
                else:

                    # return(print) mpg
                    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()
\$\endgroup\$
  • 4
    \$\begingroup\$ What makes a good question? suggests you start with a description of the problem which the code is trying to solve: for example, "This is a school assignment to convert Centigrade to Fahrenheit"; then the code; then the aspects of the code you feel unsure of: for example, whatever the professor said about it. \$\endgroup\$ – ChrisW Feb 1 '14 at 3:44
  • 4
    \$\begingroup\$ +1 for Much improved question... question for you though.... why is there some gas mileage pseudocode (off topic for Code Review) as well as real code for temperature conversion? \$\endgroup\$ – rolfl Feb 1 '14 at 4:06
  • 3
    \$\begingroup\$ For the pseudocode, there is no right answer... it's pseudocode.... but, I would put all the comment-marks '#' in the first column.... which would make it more readable. \$\endgroup\$ – rolfl Feb 1 '14 at 4:10
  • \$\begingroup\$ Wikipedia shows some examples of pseudocode. 'IF' is at the beginning of the line. After each 'IF' is a description of the test. On the next line and indented is the action taken if the test is true: for example, IF foo is not a number[newline][indent]PRINT error[newline][indent]RETURN[newline][no-indent]. The first word of each line is similar to a BASIC keyword, for example IF, SET, PRINT, RETURN, FOR. \$\endgroup\$ – ChrisW Feb 1 '14 at 5:54
3
\$\begingroup\$

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.

That confuses me too; because well-formed usually means "syntactically correct".

Because you said "no exceptions" (and previously mentioned 'pseudocode'), perhaps what you are supposed to be learning at the moment is "structured programming".

If so, IMO that would mean you should be practising the basics of control flow: "if" statements, 'loops' (like "for" and "while"), calling subroutines ("functions"), returning from subroutines, etc.

It can be argued (for example, it says so here) that "exceptions" and "structured programming" don't go well together.

For example, here is some 'structured' code (pseudocode):

OPEN inputfile
WHILE not end of input file
    READ next line of text from inputfile
    CALL subroutine to handle the line of text
CLOSE inputfile

That looks good. But, what happens if the 'CALL subroutine' throws an exception? Execution would fly out of the loop: and then the inputfile is never closed!

If the software can throw exceptions, there's a way to handle them as described here for example. However, perhaps that's more advanced than the teacher wants to be explaining and expecting of every student at the moment.

(My first two Math lectures at university were on the subject of "one plus one", "subtraction", and what happens when you multiply by zero).

Wikipedia claims that Python supports structured programming:

Python is a multi-paradigm programming language: object-oriented programming and structured programming are fully supported, and there are a number of language features which support functional programming and aspect-oriented programming (including by metaprogramming[26] and by magic methods).

Googling for structured programming and python I found this web page: Structured Programming in Python. Perhaps these (this subset of Python) are the kinds of things you will be learning in this course, and/or the style of Python programming which the teacher imagines you should use.

Although the page is quite long, note that the keyword "try" appears only once (quoted as follows) on this whole page; and note well the "TODO" comment: which shows that the author/teacher hasn't included how to use "try", nor what "try" means, in this syllabus.

[TODO: explain ValueError exception]

def concordance(word, context):
    "Generate a concordance for the word with the specified context window"
    for sent in nltk.corpus.brown.sents(categories='a'):
        try:
            pos = sent.index(word)
            left = ' '.join(sent[:pos])
            right = ' '.join(sent[pos+1:])
            print '%*s %s %-*s' %\
                (context, left[-context:], word, context, right[:context])
        except ValueError:
            pass
\$\endgroup\$
8
\$\begingroup\$

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 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()
\$\endgroup\$
  • 1
    \$\begingroup\$ Can you try to address the teacher's comment in the OP, i.e. compare this solution with another which doesn't use try/except? \$\endgroup\$ – ChrisW Feb 1 '14 at 14:47
  • \$\begingroup\$ @ChrisW Done, in Rev 4. \$\endgroup\$ – 200_success Feb 1 '14 at 17:34
  • 1
    \$\begingroup\$ Thank you. Given that avoiding an exception is more complicated than exception-handling (i.e. it requires parsing/pre-validating the input string) perhaps, who knows, the teacher expected a simplistic program which doesn't validate its input. python.org/doc//current/tutorial/… shows using an exception to parse an int. stackoverflow.com/q/2262333/49942 suggests that exception handling is the idiomatic way. \$\endgroup\$ – ChrisW Feb 1 '14 at 19:14
  • 1
    \$\begingroup\$ Thank you all for helping me out. It basically solved all of my problems with this situation. I was sure that it wasn't the "best" way (I know little about python, and didn't have much time), but sub-optimal !== "not well formed". She gave no other feedback except she said that when we do cover this in the textbook she still doesn't want use using it in our code. It makes me wonder if it makes it harder for her to navigate and if so, why is she teaching the class? \$\endgroup\$ – user2921414 Feb 3 '14 at 1:29
  • \$\begingroup\$ @user2921414 Some things do make your code harder to read. One thing is 'deeply nested if': see how your indentation keeps increasing and increasing off to the right, and see also this blog entry. If your 'get input' were a subroutine e.g. as shown in 200_success's ask function then main is straight-forward and hardly indents at all. Try to create subroutines especially when the functionality is in more than one place e.g. two places where you 'ask' for input. \$\endgroup\$ – ChrisW Feb 3 '14 at 11:43
0
\$\begingroup\$

I don't think try except else is a standard python construction - does it even execute? In that sense this does not look 'well formed'. Does this execute? It does not work when copy pasted but I think that's just formatting issues.

It's hard to write this without some try's, since the standard string > float conversion method is

 dist = input("enter distance")
 dist_num = float(dist)

which will except if dist is not a number; most python coders just try/catch around it. Without try's you'd have to provide a way of checking for floats that doesn't require a try/catch; maybe it's the point of the assignment? Apart from input validation, it's just doing a division otherwise.

To get a float from a string without a try, you could do:

def is_number(input_string):
    for char in input_string: 
        if char not in '0123456789.': return False
    return True

before the string is converted to a float (this will also filter out negative numbers for free as written)

In terms of style and construction, the conditions for the mileage and consumption are the same: positive numbers > 0. The only differences are the prompts and error messages. So it makes sense to write one function and pass the prompts in to it twice:

def get_input(prompt, error_msg ):
    value = input(prompt)
    if not is_number(value):
       print error_msg
       return get_input(prompt, error_msg)

    num = float(value)
    if num <= 0:
       print error_msg
       return get_input(prompt, error_msg)

    return num

This will just keep asking until the user gives a positive number. The rest is easy:

 miles = get_input("Enter distance traveled(in miles):", "Distance traveled must be a number larger than 0")
 gas = get_input("Enter gasoline consumed (in gallons):", "Gasoline consumed must be a number larger than 0")

 mpg = miles/gas

This could actually be improved further by replacing 'is_number' with a function that did the float conversion and converted all non-numeric values to -1; then you could just have a single test in the get_input function. I did it 'the long way' so the logic is clearer.

The last thing to look at is the layout, it would typically look like

def is_number():
    ..... etc

def get_input():
    ..... etc

def main():
    miles = get_input("Enter distance traveled(in miles):", "Distance traveled must be a number larger than 0")
   gas = get_input("Enter gasoline consumed (in gallons):", "Gasoline consumed must be a number larger than 0")
    ....... etc

That would make it easier to reuse the pieces. The standard pythonic way to call a main function is

if __name__ == 'main':
   main()

This makes sure the code only fires when the file is executed as a script, not when it's is imported as a module.

\$\endgroup\$
  • 2
    \$\begingroup\$ try-except-else is a perfectly legal Python construct, and it's being used correctly here. \$\endgroup\$ – 200_success Feb 1 '14 at 8:18
  • \$\begingroup\$ +1 for trying to answer the specific question, about how to try to code this without using try/except. \$\endgroup\$ – ChrisW Feb 1 '14 at 17:03

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.