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