Is it bad form to break out of Range-based for

Question

I have a range-based for that operates over a std::vector of objects (items), like so:

for(auto item : itemCollection)
{
   if(!item.some_check(...))
   {
      // do 5% stuff
      break;
   }

   // do the 95% stuff
}

Most of the time (>95%) the loop operates on the entire container, however, if some_check() returns a false (about 5% of the time), it'll break out of the loop early.

Is it considered bad form to break out of a range-based for? Should I instead use a while or a C++03-style for(...;...;...) here?

score 2 · Answer 1 · answered Feb 04 '15 at 05:38

No, it is never bad form to break out of any loop early unless you are aware of an invariant that will always break early. I have seen loops process the first element only:

for (auto item : itemCollection) {
  // do the 95% stuff
  break;
}

This code should never be used, unfortunately I have seen it in production code several times in the past. Of course this prompted a code review.

Anyway, this does not appear to be your case. I would still reconsider the design of your algorithm though. Why break out of the loop just because one element somewhere in the container fails a check? Why not skip that element? Maybe looking at the big picture will reveal a better way of doing whatever you are trying to do. It is difficult for me to give more guidance without more of the code and the overall design.

score 0 · Answer 2 · answered Feb 13 '15 at 01:24

I'd at least be tempted to do something like:

auto pos = std::find_if(items.begin(), items.end(), 
      [](item const &i) { return i.check(); });

std::foreach(items.begin(), pos, [](item const &i) { /* 95% stuff */ });

if (pos != items.end())
    pos->rare_stuff();

This may (or may not) be marginally longer, but to me the intent seems more clear.

Is it bad form to break out of Range-based for

2 Answers2