Cleaning Nested Try / Excepts

Question

I just wrote a piece of code that amazes me as much more nested than optimal. I would like to advise on how to improve the style of this, especially in order to make it more consistent 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 sentences below, here is 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

The main problem is that your try suggestions are too wide, especially the most distant ones: you will sooner or later encounter a mysterious mistake because one of your attempts / exceptions accidentally hid an unexpected bubble exception from some other the function you are calling.

Therefore, I would 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 also apply “flat better than nested” in a different way (independent of any try / except), that “if I have nothing more to do on this leg of the loop, continue [ie move on to the next leg of the loop] instead of “if I need to do something,” followed by a significant amount of nested code. I always preferred this style (if / continue or if / return) for nested if in languages ​​that supply functions such as continue (by essentially all modern, since C has this ;-).

But this is a simple “flat versus nested” style, and the problem is that you keep your suggestions. In the worst case scenario, when you simply cannot just continue or return in the except clause, you can use try / except / else: put in the try clause only what absolutely SHOULD be there - a tiny piece of code that is likely to be raised - and add the rest of the following code (the part that is NOT allowed and should not be raised) in the else clause. This does not change nesting, but it makes a huge difference in reducing the risk of accidentally hiding exceptions that are NOT expected!

Answer 2

I wonder if the jobs for each unit of time are really broken, will they raise an AttibuteError or some other exception? In particular, if there is something about a job that is really busted, you probably shouldn't catch them.

Another option that might help is to wrap only the violation code with try-catch, putting the exception handler as close to the exception as possible. Here is a hit:

 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, (), ()) 

Please note that there is only one exception handler that we handle by ignoring correctly. since we can predict the possibility that there is not a single attribute, we check it explicitly, which helps make the code more understandable. Otherwise, it is not really obvious why you are catching an AttributeError or even raising it.

Answer 3

You can create a function that executes the basic logic and another function that calls this function by wrapping it with try ... except statements. Then, in the main application, you can simply call those functions that already handle exceptions. (Based on the recommendations of the book "Clean Code").

Answer 4

Well, the trick here is to find out if they are broken. This is the handling of the exception handling part. I mean, at the very least, to print a warning that talks about the commentary assumption. Worrying about excessive nesting before the actual treatment looks like you are ahead of yourself. Worrying about being right before being stylish.