isocpp / isocpp/CppCoreGuidelines

Update example in ES.77 Minimize the use of break and continue in loops, to adhere to rule F.56

Open
#1,765 1 comment 0 reactions 1 assignee View on GitHub

@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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.