Avoid continue

“I have never seen a piece of code that was not improved by refactoring it to remove the continue statement.” —Douglas Crockford, JavaScript: The Good Parts (page 111)

The continue statement is terrible.

For one thing, consider what “continue” means in English. It means keep going. It means don’t stop. It means “persist in an activity or process”. Now consider this code:

for (int i = 0; i < 10; i++) {
    A();
    B();
    continue;
    C();
    D();
}

First we have A(), then B(), then we get to continue, which does what? It does not keep going. It does not “persist in an activity or process”. That would mean continuing to C(). Instead, it interrupts the flow of the code. It would have been more self-documenting to call it do_not_continue.

More practically, it’s effectively a goto statement and it breaks up the flow of code in a similar way. It’s too easy, when skimming code, to miss it:

for (Node node : nodeList) {
    if (node.isBad()) {
        continue;
    }

    processNode();
}

It’s also more logically difficult to parse. The reader has to think, “If it’s bad, then we continue, otherwise we process.” (See Keep if clauses side-effect free for a comically bad example of this.) Easier to instead think, “If it’s not bad, we process,” like this:

for (Node node : nodeList) {
    if (!node.isBad()) {
        processNode();
    }
}

If the content of your loop is too large or logically complex to effectively do this, then it’s too large to put in-line and should be broken out into a different function, where you can use return. For example:

for (Node node : nodeList) {
    if (node.isQuestionable()) {
        NodeData nodeData = node.getNodeData();
        if (nodeData.isBad()) {
            continue;
        }
    }

    processNode();
}

Here the continue is even more hidden than in the first example, but it’s harder to simply invert the condition. You can refactor in two ways. The first uses return:

for (Node node : nodeList) {
    potentiallyProcessNode(node);
}

void potentiallyProcessNode(Node node) {
    if (node.isQuestionable()) {
        NodeData nodeData = node.getNodeData();
        if (nodeData.isBad()) {
            return;
        }
    }

    processNode();
}

Even better is to factor out the investigation:

for (Node node : nodeList) {
    if (shouldProcessNode()) {
        processNode();
    }
}

boolean shouldProcessNode(Node node) {
    if (node.isQuestionable()) {
        NodeData nodeData = node.getNodeData();
        if (nodeData.isBad()) {
            return false;
        }
    }

    return true;
}

Breaking it out into a separate function has the additional advantages of allowing you to name it and document it, and perhaps unit test it if you’re into that.

But isn’t return also a goto? Yes, but it’s much clearer where it’s going: it’s completely leaving the area. You don’t have to look carefully at the enclosing blocks until you find a loop that’s being continued.

To see the bug risks, consider this code:

for (Node node : nodeList) {
    // bunch of stuff.
    // many lines here.

    log("considered node", node);
}

Now someone adds this guard:

for (Node node : nodeList) {
    if (node.isBad()) {
        continue;
    }

    // bunch of stuff.
    // many lines here.

    log("considered node", node);
}

They didn’t notice the log() at the end and accidentally skipped over it. Had they instead written a long if statement, they more likely would have noticed it when indenting and considering which lines should be inside and outside the if block. The reverse can happen too, where a continue already exists somewhere in the body and someone adds a line at the end. I’ve seen both of these happen.

And finally, what about break? It’s not awesome, but has two advantages over continue. For one, it’s not named the literal inverse of what it does. And secondly, to avoid a break requires a bunch of extra logic:

boolean done = false;
for (int i = 0; i < 10 && !done; i++) {
    if (wantToBeDone(i)) {
        done = true;
    } else {
        // do something
    }
}

That boolean adds complexity, I think more complexity than removing the break saves. And that approach doesn’t work at all with for-in loops:

for (Node node : nodeList) {
    if (wantToBeDone(node)) {
        // ?!?
    } else {
        // do something
    }
}

So yeah avoid the break if it’s easy, but continue is the true bad guy here.