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?
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.
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.