I have a for loop in python like this:

for item in items:
    onwards = make_flow_decision(item)
    if onwards == 'break':
        break
    elif onwards == 'continue':
        continue
    elif onwards == 'carry_on':
        process_the_item(item)

So idea is that flow control for the loop is factored out because it is really complex.

Is there a better way of doing this than passing strings around and checking for special strings like 'continue', 'break', and 'carry_on'?

Looking for ideas generally that make this a little more elegant.

  • Can it be split into should_break() and should_continue() methods? (assuming you don't have any genuinely meaningful names you could use like if unauthenticated() break elif not_eligible_for_coupon() continue) – Ixrec Aug 8 '15 at 16:47
  • Theoretically yes, but that'd push the ugliness to two flow decision methods. Not sure it would help too much, at least with this code. – mlissner Aug 8 '15 at 16:51
  • I guess it depends on what you consider ugly. My suggestion was meant to remove the need to return those magic strings. I suppose the other easy answer is that the continue cases can be filter()ed out and the "normal" case can be map()ed, but the break may be tricky depending on how much you care about leaving some but not all items processed at the end. – Ixrec Aug 8 '15 at 16:57
  • This gives me one idea. I could rewrite it so that the make_flow_decision() was cheap to check once a break was needed. Then, instead of actually breaking out of the loop, I could just continue through it until it's done. It'd mean needless iteration, but if cheap enough that's not so bad. The resulting code could be simplified to, if press_on: process_the_item(). And the check would just emulate a break by continuing until the last item. – mlissner Aug 8 '15 at 17:57
  • Instead of strings, why not use enums? – coredump Aug 8 '15 at 18:57

What you could do, is factor out the filtering and breaking into a generator, which can be really handy if that logic is used multiple times, with different processing in the middle.

def relevant_items(items):
    for item in items:
        # logic to continue or break/return goes here
        yield item

def process_many_items(items):
    for item in relevant_items(items):
        process_item(item)

This does however add an extra layer of indirection, which is probably only worth it if the relevant_items method would make sense outside process_many_items.

  • Indeed, relevant_items does make more sense outside of the processing loop. I dig this approach. Will give it a go. – mlissner Aug 11 '15 at 14:26
  • Actually, I take it back! Darn. This won't work because some processing of the item has to happen before the continue/break/return decision can be made. So the loop is more like, for item in items, calculate some stuff, using that stuff, make a decision, using that decision, continue/break/carry_on. – mlissner Aug 11 '15 at 14:28

You can use enumerated types instead of strings, and keep your implementation.

Or, if you don't mind using exceptions you can write:

try:
    for item in items:
        if shall_process(item):
            process_the_item(item)
except StopIteration:
    pass

Here, shall_process returns a boolean and can throw an exception to exit the iteration.

  • Not a bad approach. The logic feels a little oblique though. Like, it's much easier to understand the flow control of my first example than it is to figure out that shall_process is emulating a break by throwing an exception. The one thing that might help here though is the StopIteration exception. The enums approach seems a little less confusing though, I'll say that. – mlissner Aug 8 '15 at 23:45

Your Answer

 

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

Not the answer you're looking for? Browse other questions tagged or ask your own question.