0

Relevant code:

def return_t_done(queryList):
    for item in list:
        if item[0] == "t_done":
            return int(item[1])
    raise Error("Attempted to get t_done's value, yet no t_done exists")

So basically this code runs through a nested list in the form of

[[spam,eggs],[ham,yum]]

and looks for a field ([[field,value],[field,value]]) of 't_done' and, when it finds it, returns the value associate with that field. There should always be a t_done field, but in case there isn't (this is parsing automatically generated log files using an app I didn't write and can't access the source code for, so who knows what could go wrong) I would like to elegantly raise an exception in the most appropriate way possible.

That said, this seems like the most elegant way to me, but I'm not particularly versed in Python, and my research into the docs confused me a bit. So, should I wrap the for loop in a try, except clause? Should I write a class for this error? Should I raise an exception instead of an error? Is it okay as written?

Thanks!

2
  • Looks fine to me. The type of exception you should raise is debatable. Commented Jul 2, 2013 at 14:59
  • You should probably be raising ValueError or LookupError here Commented Aug 26, 2013 at 11:50

2 Answers 2

4

The way you have it is fine. But one other possibility is to use the little-known else cause for the for loop. That is evaluated if and only if the for loop completes successfully.

for item in list:
    if item[0] == "t_done":
        return int(item[1])
else:
    raise Error(...)

Note the indentation: the else lines up with the for, not the if.

Sign up to request clarification or add additional context in comments.

1 Comment

Thanks. Is there a reason to add an extra line besides the subjective readability enhancement? Is there precedent?
0

This code is good. There's no need for a try/except structure.

Or at least, there's no need for a try/except in this function. But depending on the situation, there should probably be a try/except somewhere else to catch the exception. You'll want to put that in the code that calls this function. (Or maybe in the code that calls that code, or some level above that).

For instance, lets say that when you can't find the correct key in the logs, you want to use a default value:

def main():
    query_list = fetch_query_list()

    try:
        my_integer = return_t_done(query_list)
    except Error:
        my_integer = 0 # Default value

    some_more_code_here(my_integer)

Or maybe you want to shut down the whole script, in which case:

    try:
        my_integer = return_t_done(query_list)
    except Error as err:
        print("There was a problem:")
        print(err)
        print("shutting down")
        exit(1)

That's the whole idea of exceptions: If a function is raising an exception, then it's usually because that function knows that something has gone wrong, but it doesn't have any idea what to do about it. So it just packages up as much information as it can into an exception, and then it passes that exception back up the food-chain to whoever called the function in the first place. Then it keeps getting passed up the chain until hopefully, it reaches someone who knows what to do about it


Side note: if every item in the list is guaranteed to have 2 entries, then you can do a thing called destructuring, and I think it often makes the code easier to read:

    for field_name, value in queryList:
        if field_name == "t_done":
            return int(value)

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.