1

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.

Improve this question
5
  • 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) Commented Aug 8, 2015 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. Commented Aug 8, 2015 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. Commented Aug 8, 2015 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. Commented Aug 8, 2015 at 17:57
  • Instead of strings, why not use enums? Commented Aug 8, 2015 at 18:57

2 Answers 2

Reset to default
5

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.

Improve this answer
2
  • Indeed, relevant_items does make more sense outside of the processing loop. I dig this approach. Will give it a go. Commented Aug 11, 2015 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. Commented Aug 11, 2015 at 14:28
2

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.

Improve this answer
1
  • 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. Commented Aug 8, 2015 at 23:45

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.