Are `break` and `continue` bad programming practices?

Question

My boss keeps mentioning nonchalantly that bad programmers use break and continue in loops.

I use them all the time because they make sense; let me show you the inspiration:

function verify(object) {
    if (object->value < 0) return false;
    if (object->value > object->max_value) return false;
    if (object->name == "") return false;
    ...
}

The point here is that first the function checks that the conditions are correct, then executes the actual functionality. IMO same applies with loops:

while (primary_condition) {
    if (loop_count > 1000) break;
    if (time_exect > 3600) break;
    if (this->data == "undefined") continue;
    if (this->skip == true) continue;
    ...
}

I think this makes it easier to read & debug; but I also don't see a downside.

Answer 1

When used at the start of a block, as first checks made, they act like preconditions, so it's good.

When used in the middle of the block, with some code around, they act like hidden traps, so it's bad.

Answer 2

You could read Donald Knuth's 1974 paper Structured Programming with go to Statements, in which he discusses various uses of the go to that are structurally desirable. They include the equivalent of break and continue statements (many of the uses of go to in there have been developed into more limited constructs). Is your boss the type to call Knuth a bad programmer?

(The examples given interest me. Typically, break and continue are disliked by people who like one entry and one exit from any piece of code, and that sort of person also frowns on multiple return statements.)

Answer 3

I do not believe they are bad. The idea that they are bad comes from the days of structured programming. It is related to the notion that a function must have a single entry point and a single exit point, i. e. only one return per function.

This makes some sense if your function is long, and if you have multiple nested loops. However, your functions should be short, and you should wrap loops and their bodies into short functions of their own. Generally, forcing a function to have a single exit point can result in very convoluted logic.

If your function is very short, if you have a single loop, or at worst two nested loops, and if the loop body is very short, then it is very clear what a break or a continue does. It is also clear what multiple return statements do.

These issues are addressed in "Clean Code" by Robert C. Martin and in "Refactoring" by Martin Fowler.

Answer 4

Bad programmers speak in absolutes (just like Sith). Good programmers use the clearest solution possible (all other things being equal).

Using break and continue frequently makes code hard to follow. But if replacing them makes the code even harder to follow, then that's a bad change.

The example you gave is definitely a situation where the breaks and continues should be replaced with something more elegant.

Answer 5

Most people think it's a bad idea because the behaviour isn't easily predictable. If you're reading through the code and you see while(x < 1000){} you assume it's going to run until x >= 1000...But if there are breaks in the middle, then that doesn't hold true, so you can't really trust your looping...

It's the same reason people don't like GOTO: sure, it can be used well, but it can also lead to godawful spaghetti code, where the code leaps randomly from section to section.

For myself, if I was going to do a loop that broke on more than one condition, I'd do while(x){} then toggle X to false when I needed to break out. The final result would be the same, and anyone reading through the code would know to look more closely at things that switched the value of X.

Answer 6

Yes you can [re]write programs without break statements (or returns from the middle of loops, which do the same thing). But you may have to introduce additional variables and/or code duplication both of which typically make the program harder to understand. Pascal (the programming language) was very bad especially for beginner programmers for that reason. Your boss basically wants you to program in Pascal's control structures. If Linus Torvalds were in your shoes, he would probably show your boss the middle finger!

There's a computer science result called the Kosaraju's hierarchy of control structures, which dates back to 1973 and which is mentioned in Knuth's (more) famous paper on gotos from 1974. (This paper of Knuth was already recommended above by David Thornley, by the way.) What S. Rao Kosaraju proved in 1973 is that it's not possible to rewrite all programs that have multi-level breaks of depth n into programs with break depth less than n without introducing extra variables. But let's say that's just a purely theoretical result. (Just add a few extra variables?! Surely you can do that to please your boss...)

What's far more important from a software engineering perspective is a more recent, 1995 paper by Eric S. Roberts titled Loop Exits and Structured Programming: Reopening the Debate (http://cs.stanford.edu/people/eroberts/papers/SIGCSE-1995/LoopExits.pdf). Roberts summarizes several empirical studies conducted by others before him. For example, when a group of CS101-type students were asked to write code for a function implementing a sequential search in an array, the author of the study said the following about those students who used a break/return/goto to exit the from the sequential search loop when the element was found:

I have yet to find a single person who attempted a program using [this style] who produced an incorrect solution.

Roberts also says that:

Students who attempted to solve the problem without using an explicit return from the for loop fared much less well: only seven of the 42 students attempting this strategy managed to generate correct solutions. That figure represents a success rate of less than 20%.

Yes, you may be more experienced than CS101 students, but without using the break statement (or equivalently return/goto from the middle of loops), eventually you'll write code that while nominally being nicely structured is hairy enough in terms of extra logic variables and code duplication that someone, probably yourself, will put logic bugs in it while trying to follow your boss' coding style.

I'm also gonna say here that Roberts' paper is far more accessible to the average programmer, so a better first read than Knuth's. It's also shorter and covers a narrower topic. You could probably even recommend it to your boss, even if he is the management rather than the CS type.

Answer 7

I don't consider using either of these bad practice, but using them too much within the same loop should warrant rethinking the logic being used in the loop. Use them sparingly.

Answer 8

The example you gave doesn't need breaks nor continues:

while (primary-condition AND
       loop-count <= 1000 AND
       time-exec <= 3600) {
   when (data != "undefined" AND
           NOT skip)
      do-something-useful;
   }

My ‘problem’ with the 4 lines in your example is that they are all on the same level but they do different things: some break, some continue... You have to read each line.

In my nested approach, the more deeper you go, the more ‘useful‘ the code becomes.

But, if deep inside you'd find a reason to stop the loop (other than primary-condition), a break or return would have it's use. I'd prefer that over the use of an extra flag that is to be tested in the top-level condition. The break/return is more direct; it better states the intent than setting yet another variable.

Answer 9

The "badness" is dependent on how you use them. I typically use breaks in looping constructs ONLY when it will save me cycles that can't be saved through a refactoring of an algorithm. For instance, cycling through a collection looking for an item with a value in a specific property set to true. If all you need to know is that one of the items had this property set to true, once you achieve that result, a break is good to terminate the loop appropriately.

If using a break won't make the code specifically easier to read, shorter to run or save cycles in processing in a significant manner, then it's best not to use them. I tend to code to the "lowest common denominator" when possible to make sure that anyone who follows me can easily look at my code and figure out what's going on (I am not always successful at this). Breaks reduce that because they do introduce odd entry/exit points. Misused they can behave very much like an out of whack "goto" statement.

Answer 10

The essential notion comes from being able to semantically analyze your program. If you have a single entry and a single exit, the math needed to denote possible states is considerably easier than if you have to manage forking paths.

In part, this difficulty reflects out into being able to conceptually reason about your code.

Frankly, your second code is not obvious. What is it doing? Does continue 'continue', or does it 'next' the loop? I have no idea. At least your first example is clear.

Answer 11

Absolutely not... Yes the use of goto is bad because it deteriorates the structure of your program and also it is very difficult to understand the control flow.

But use of statements like break and continue are absolutely necessary these days and not considered as bad programming practice at all.

And also not that difficult to understand the control flow in use of break and continue. In constructs like switch the break statement is absolutely necessary.

Answer 12

I disagree with your boss. There are proper places for break and continue to be used. In fact the reason that execeptions and exception handling were introduced to modern programming languages is that you can't solve every problem using just structured techniques.

On a side note I don't want to start a religious discussion here but you could restructure your code to be even more readable like this:

while (primary_condition) {
    if (loop_count > 1000) || (time_exect > 3600) {
        break;
    } else if ( ( this->data != "undefined") && ( !this->skip ) ) {
       ... // where the real work of the loop happens
    }
}

On another side note

I personally dislike the use of ( flag == true ) in conditionals because if the variable is a boolean already, then you are introducing an additional comparison that needs to happen when the value of the boolean has the answer you want - unless of course you are certain that your compiler will optimize that extra comparison away.

Answer 13

I would replace your second code snippet with

while (primary_condition && (loop_count <= 1000 && time_exect <= 3600)) {
    if (this->data != "undefined" && this->skip != true) {
        ..
    }
}

not for any reasons of terseness - I actually think this is easier to read and for someone to understand what is going on. Generally speaking the conditions for your loops should be contained purely within those loop conditions not littered throughout the body. However there are some situations where break and continue can help readability. break moreso than continue I might add :D

Answer 14

I agree with your boss. They are bad because they produce methods with high cyclomatic complexity. Such methods are difficult to read and difficult to test. Fortunately there's an easy solution. Extract the loop body into a separate method, where the "continue" becomes "return". "Return" is better because after "return" it's over -- there's no worries about the local state.

For "break" extract the loop itself into a separate method, replacing "break" with "return".

If the extracted methods require a large number of arguments, that's an indication to extract a class -- either collect them into a context object.

Answer 15

I will just quote from Code Complete:

Use continue for tests at the top of the loop. A good use of continue is for moving execution past the body of the loop after testing a condition at the top. For example, if the loop reads records, discards records of one kind, and processes records of another kind you could put a test like this at the top of the loop. Using continue in this way lets you avoid an if test that would effectively indent the entire body of the loop. If on the other hand, the continue occurs toward the middle or end of the loop, use and if instead.

Answer 16

I think it's only a problem when nested deeply inside multiple loops. It's hard to know which loop you are breaking to. It might be difficult to follow a continue also, but I think the real pain comes from breaks - the logic can be difficult to follow.

Answer 17

As long as they're not used as disguised goto like in the following example :

do
{
      if (foo)
      {
             /*** code ***/
             break;
      }

      if (bar)
      {
             /*** code ***/
             break;
      }
} while (0);

I'm fine with them. (Example seen in production code, meh)

Answer 18

I don't like either of these styles. Here's what I would prefer:

function verify(object)
{
    if not (object->value < 0) 
       and not(object->value > object->max_value)
       and not(object->name == "") 
       {
         do somethign important
       }
    else return false; //probably not necessary since this function doesn't even seem to be defined to return anything...?
}

I really don't like using return to abort a function. It feels like an abuse of return.

Using break also is not always clear to read.

Better yet might be:

notdone := primarycondition    
while (notDone)
{
    if (loop_count > 1000) or (time_exect > 3600)
    {
       notDone := false; 
    }
    else
    { 
        skipCurrentIteration := (this->data == "undefined") or (this->skip == true) 

        if not skipCurrentIteration
        {
           do something
        } 
        ...
    }
}

less nesting and the complex conditions are refactored into variables (in a real program you'd have to have better names, obviously...)

(All above code is pseudo-code)

Answer 19

No. It's a way to solve a problem, and there are other ways to solve it.

Many current mainstream languages (Java, .NET (C# + VB), PHP, write your own) use "break" and "continue" to skip loops. They both "structured goto (s)" sentences.

Without them:

String myKey = "mars";

int i = 0; bool found = false;
while ((i < MyList.Count) && (not found)) {
  found = (MyList[i].key == myKey);
  i++;   
}
if (found)
  ShowMessage("Key is " + i.toString());
else
  ShowMessage("Not found.");

With them:

String myKey = "mars";

for (i = 0; i < MyList.Count; i++) {
  if (MyList[i].key == myKey)
    break;
}
ShowMessage("Key is " + i.toString());

Note that, "break" and "continue" code is shorter, and usually turns "while" sentences into "for" or "foreach" sentences.

Both cases are a matter of coding style. I prefer not to used them, because the verbose style allows me to have more control of the code.

I actually, work in some projects, where it was mandatory to use those sentences.

Some developers may think they are not necesarilly, but hypothetical, if we had to remove them, we have to remove "while" and "do while" ("repeat until", you pascal guys) also ;-)

Conclusion, even if I prefer not to use them, I think its an option, not a bad programming practice.

Answer 20

I'm not against continue and break in principle, but I think they are very low-level constructs that very often can be replaced by something even better.

I'm using C# as an example here, consider the case of wanting to iterate over a collection, but we only want the elements that fulfil some predicate, and we don't want to do any more than a maximum of 100 iterations.

for (var i = 0; i < collection.Count; i++)
{
    if (!Predicate(item)) continue;
    if (i >= 100) break; // at first I used a > here which is a bug. another good thing about the more declarative style!

    DoStuff(item);
}

This looks REASONABLY clean. It's not very hard to understand. I think it would stand to gain a lot from being more declarative though. Compare it to the following:

foreach (var item in collection.Where(Predicate).Take(100))
    DoStuff(item);

Maybe the Where and Take calls shouldn't even be in this method. Maybe this filtering should be done BEFORE the collection is passed to this method. Anyway, by moving away from low-level stuff and focusing more on the actual business logic, it becomes more clear what we're ACTUALLY interested in. It becomes easier to separate our code into cohesive modules that adhere more to good design practices and so on.

Low-level stuff will still exist in some parts of the code, but we want to hide this as much as possible, because it takes mental energy that we could be using to reason about the business problems instead.

Answer 21

Code Complete has a nice section about using goto and multiple returns from routine or loop.

In general it's not bad practice. break or continue tell exactly what happens next. And I agree with this.

Steve McConnell (author of Code Complete) uses almost the same examples as you to show advantages of using various goto statements.

However overuse break or continue could lead to complex and unmaintainable software.