Cleaning up nested Try/Excepts

Question

I've just written a chunk of code that strikes me as being far more nested than is optimal. I'd like advice on how to improve the style of this, particularly so that it conforms more with "Flat is better than nested."

for app in apps:
    if app.split('.', 1)[0] == 'zc': #only look for cron in zc apps
        try:
            a = app + '.cron'
            __import__(a)
            m = sys.modules[a]

            try:
                min = m.cron_minute()
                for job in min:
                    k.add_interval_task(job[0], 'minute task', r(M_LB, M_UB),
                                        60*job[1], 
                                        kronos.method.threaded, (), ())
            except AttributeError: #no minute tasks
                pass

            try:
                hour = m.cron_hour()
                for job in hour:
                    k.add_daytime_task(job[0], 'day task', range(1, 8), None,
                                       (job[1], r(H_LB, H_UB)), 
                                       kronos.method.threaded, (), ())
            except AttributeError: #no hour tasks
                pass

        except ImportError: #no cron jobs for this module
            pass

Edit: Combining the suggestions from below, here's my rewritten form.

for app in apps:
    if app.split('.', 1)[0] != 'zc': #only look for cron in zc apps
        continue

    try:
        a = app + '.cron'
        __import__(a)
    except ImportError: #no cron jobs for this module, continue to next one
        continue

    m = sys.modules[a]
    if hasattr(m, 'cron_minute'):
        min = m.cron_minute()
        for job in min:
            k.add_interval_task(job[0], 'minute task', r(M_LB, M_UB),
                                60*job[1], 
                                kronos.method.threaded, (), ())

    if hasattr(m, 'cron_hour'):
        hour = m.cron_hour()
        for job in hour:
            k.add_daytime_task(job[0], 'day task', range(1, 8), None,
                               (job[1], r(H_LB, H_UB)), 
                               kronos.method.threaded, (), ())

Answer 1

You can create a function that performs the main logic, and another function which invokes that function, wrapping it with the try...except statements. Then, in the main application, you can just invoke those function which already handle the exceptions. (Based on the recommendations of the "Clean Code" book).

Answer 2

Well, the trick here is to figure out if they are broken. That is the handling part of exception handling. I mean, at least print a warning that states the comment's assumption. Worrying about excessive nesting before the actual handling seems like you are getting ahead of yourself. Worry about being right before being stylish.

Answer 3

The main problem is that your try clauses are too broad, particularly the outermost one: with that kind of habit, you WILL sooner or later run into a mysterious bug because one of your try/except has accidentally hidden an unexpected exception "bubbling up" from some other function you're calling.

So I'd suggest, instead:

for app in apps:
    if app.split('.', 1)[0] != 'zc': #only look for cron in zc apps
        continue

    try:
        a = app + '.cron'
        __import__(a)
    except ImportError: #no cron jobs for this module
        continue

    # etc etc

As an aside, I'm also applying "flat is better than nested" in another way (not dependent on any try/except) which is "if I have nothing more to do on this leg of the loop, continue [i.e. move on to the next leg of the loop] instead of "if I have something to do:" followed by a substantial amount of nested code. I've always preferred this style (of if/continue or if/return) to nested if's in languages that supply functionality such as continue (essentially all modern ones, since C has it;-).

But this is a simple "flat vs nested" style preference, and the meat of the issue is: keep your try clauses small! Worst case, when you just can't simply continue or return in the except clause, you can use try/except/else: put in the try clause only what absolutely MUST be there -- the tiny piece of code that's likely and expected to raise -- and put the rest of the following code (the part that's NOT supposed nor expected to raise) in the else clause. This doesn't change the nesting, but DOES make a huge difference in lowering the risk of accidentally hiding exceptions that are NOT expected!

Answer 4

I wonder, if the jobs for each time unit is actually broken, would they raise an AttibuteError, or some other exception? In particular, if there's something about a job that is really busted, you probably aught not to catch them.

Another option that can help is to wrap only the offending code with a try-catch, putting the exception handler as close to the exception as possible. Here's a stab:

for app in apps:
    if app.split('.', 1)[0] == 'zc': #only look for cron in zc apps
        try:
            a = app + '.cron'
            __import__(a)
            m = sys.modules[a]
        except ImportError: #no cron jobs for this module
                            #exception is silently ignored
                            #since no jobs is not an error
            continue
        if hasattr(m, "cron_minute"):
            min = m.cron_minute()
            for job in min:
                k.add_interval_task(job[0], 'minute task', r(M_LB, M_UB),
                                    60*job[1], 
                                    kronos.method.threaded, (), ())

        if hasattr(m, "cron_hour"):
            hour = m.cron_hour()
            for job in hour:
                k.add_daytime_task(job[0], 'day task', range(1, 8), None,
                                   (job[1], r(H_LB, H_UB)), 
                                   kronos.method.threaded, (), ())

notice there is only one exception handler here, which we handle by correctly ignoring. since we can predict the possibility of there not being one attribute or another, we check for it explicitly, which helps to make the code a bit clearer. Otherwise, it's not really obvious why you are catching the AttributeError, or what is even raising it.