isocpp / isocpp/CppCoreGuidelines
Update example in ES.77 Minimize the use of break and continue in loops, to adhere to rule F.56
@BjarneStroustrup is already working on this.
Since Aug 19, 2021.
- Dominant language
- CSS
- Stars
- 45.3k
- Forks
- 5.6k
- PR merge metrics
- No merged PRs in 30d
Description
Now, given that we have F.56 "Avoid unnecessary condition nesting", I suggest to somehow change or remove an example in
ES.77: Minimize the use of break and continue in loops.
Wording of today:
Often, a loop that uses continue can equivalently and as clearly be expressed by an if-statement.
for (int item : vec) { //BAD
if (item%2 == 0) continue;
if (item == 5) continue;
if (item > 10) continue;
/* do something with item */
}
for (int item : vec) { //GOOD
if (item%2 != 0 && item != 5 && item <= 10) {
/* do something with item */
}
}
I disagree that in the example above, the "loop that uses continue can equivalently and as clearly be expressed by an if-statement".
My main objections against the example:
- The original code is bad in that it contains duplicated code for no good reason. It contains 3 if-statements, when 1 is enough.
- The rewritten condition is harder to understand than the original version.
The code iterates over a vector, and there are a few exceptions where some items should be skipped. I think that the intent is obscured by rewriting the condition in an unnatural way, just to avoid a continue. Using a continue and cleaning up the condition would give an easy to understand code. Now, the following rewrite does not really fit in "avoid continue ". I think this a good way of using continue, instead of avoiding it.
// simple and easy to understand logic
for (int item : vec) {
if (item%2 == 0 || item == 5 || item > 10) // Guard clause, sort of an early return, by using continue.
continue;
/* do something with item */
}
Or would it maybe be better with just a minor change from today's wording in ES.77?
Depending on how bad you think continue is. As follows:
for (int item : vec) { //BAD
if (item%2 == 0 || item == 5 || item > 10)
continue;
/* do something with item */
}
for (int item : vec) { //GOOD
if (!(item%2 == 0 || item == 5 || item > 10)) {
/* do something with item */
}
}
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Assessment
This issue has not been assessed yet.