0

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?

2

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.

0

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.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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