8

Before I start this question I know the java 'goto' is a big no go.

So I've been writing a program and I have some indented Loops and statements and I need to BREAK multiple on command. Rather than having a load of boolean variables and if(!<booleanName>) BREAK; statements throughout these loops and statements what is everyone's opinions on using labels to break them using the BREAK <label> statement?

e.g.

 for(...) {

      indented_example_loops:     // Label name
           for(...) {
                for(...) {
                     if(match) break indented_example_loops;
                     // ^ Break statement to break the 2 for loops at once
                }
           }

 }

Perfectly okay? Okay to do occasionally? Completely avoid? or should i go to a corner and call the Devil to take my soul?

9
  • 5
    I for one think your problem is the deep nesting, not a use or abuse of labels and goto Feb 5, 2013 at 9:26
  • The nesting is necessary as its lists nested inside lists i.e. Group > Member > Emails/Addresses/Phone-Numbers
    – Skepi
    Feb 5, 2013 at 9:37
  • 8
    Put the nested code in a separate method and use return instead of break. Structured programming was invented more than 40 years ago (en.wikipedia.org/wiki/Structured_program_theorem) for good reasons.
    – Giorgio
    Feb 5, 2013 at 10:17
  • 2
    @Giorgio, you should make that an answer. Feb 5, 2013 at 15:20
  • 3
    break, continue, throw, switch, and return are all forms of the much maligned goto. Some of this is warranted, some is not. Labels can make these structures easier to read or harder. Treat them all with respect.
    – user40980
    Feb 5, 2013 at 23:27

4 Answers 4

23

It's all about readability and ease of understanding.

The easiest to understand loops are those with simple end conditions and no breaks at all. So if you can write your loops like that, do it.

while (someCondition) {
  // do stuff
}

Next comes the loop with a simple end condition and a single break:

while (someCondition) {
  // do stuff
  if (otherCondition) {
    break;
  }
  // maybe some more stuff
}

Everything that's more complex than this is a potential problem. A break with a label is always tricky, because you'll have to hunt for the label to find out which statement/block the break escapes from (i.e. the break will continue after the labeled block/statement).

The worst kind (the one you almost always want to avoid) is a nested loop where you break somewhere in the middle.

while (someCondition) {
  myLabel:
  for (Foo foo : getAllFoos()) {
    for (Bar bar : foo.getBars()) {
      // do stuff
      if (someCondition(bar)) {
        break myLabel; // BAD!
      }
      // some more stuff
    }
  }
}

The problem here is that the actual flow of processing becomes really hard to follow. If you start having code like this, then you should definitely refactor it.

A common refactoring is to extract one (or more) nested loops into a method and turning the break into a return. This has the added benefit of giving a name to that piece of the code. If the name is chosen well, then that will help greatly in understanding the code.

3
  • Thanks, ill take this into account. I guess using methods and return statements would give me the same result and be a lot easier to read
    – Skepi
    Feb 5, 2013 at 9:38
  • +1 even if I dislike break in any case (except switch).
    – Uwe Plonus
    Feb 5, 2013 at 9:45
  • 1
    @UwePlonus: I think on the "low end" (simple breaks), it's mostly a matter of taste, and I won't argue against avoiding it altogether. The most important part (in my opinion) is to avoid the really hard to read constructs with nested loops and multiple break labels. Those are mind-benders and not usually worth the trouble. Feb 5, 2013 at 9:54
6

You are opening yourself up to spaghetti code

While the language allows you to do this, it is not good practice. There are many alternative looping structures that will achieve the same objective and make your code much more readable.

Essentially, labels and gotos are a holdover from assembly language code (which is all about labels and gotos). It really has no place in higher level languages.

2
  • What looping structures would you advise? Why do you think a break is a goto?
    – Florian F
    Sep 10, 2014 at 19:08
  • The approach that @JoachimSauer has given is the correct way to implement this. The break is going to an earlier label which is not as readable as, for example, a return statement from a method.
    – Gary
    Sep 11, 2014 at 12:29
1

Separate the iteration and the processing by encapsulating the iteration.

Iterating over multi-level structures with nested loops leads to applications where most of the code is looping noise. To process all the Level2Things in a Level0Thing, replace your nested loops with this:

for (Level2Thing t2: topThing.level2Things()) {
   // do something
}

where level2Things() returns an Iterable<Level2Thing>.

You can find some sample code for encapsulating nested iterations here.

4
  • Suppose the loops were for x and y over some integer range, I don't see how that can be done in a simple way. I suspect it impacts performance.
    – Florian F
    Sep 10, 2014 at 19:16
  • @FlorianF: you build an iterator that holds values for x and y and increments y to the limit, then resets y and increments x. The performance impact will be small assuming you are doing anything interesting inside the loop. Basically the overhead is the one-time iterator allocation and the calls to hasNext() and next(). Sep 10, 2014 at 20:41
  • OK, but that means you create an Iterator that creates and returns Tuples of 2 Integers. Then you unpack the Tuple into x and y. It is not exactly simple. Unless you have many similar double-loops, it is overkill to abstract them out in an iterator. For simple cases, breaking out of 2 loops is still the best option IMHO.
    – Florian F
    Sep 11, 2014 at 13:57
  • @FlorianF: depends on your idea of simple. An iterator has a very simple interface: hasNext() and next(). So it's clear what the iterator does. The iterator itself is pretty simple too after you've written one. Or you could just use Guava. The key is to think first about the what and then worry about the how. What you are doing is iterating over some set of things. How you are doing it is by traversing a two-level structure. That how could change, so you don't want multiple copies of that nested-loop structure scattered about. Sep 11, 2014 at 15:51
-3

For very deeply nested loops or complex code flow, I often declare anonymous Runnables. While not directly related to the question, it is still relevant. I'm sure there is a better example than this, but it's a start.

public int ComplexFunction() {
    final Integer value = new Integer();
    final Boolean quit = new Boolean(false);

    final Runnable doTest = new Runnable() {
        public void run() {
            if (value.intVal() > 100) {
                quit = new Boolean(true);
            }
        }
    }


    while (true) {
        value = new Integer(value.intVal() + 5);
        doTest.run();
        if (quit.booleanVal() == true) {
            return 1;
        }
    }
    return 0;
}
2
  • 2
    this seems to be missing an example of deeply nested loop showing how recommended approach is expected to work and explanation of how it would be better than using label
    – gnat
    Sep 10, 2014 at 18:26
  • 2
    I don't see a label in that code at all. You may wish to read Oracle's tutorial on branching statements - in particular pay attention to the BreakWithLabelDemo. As an aside, please endeavor to have code that compiles and uses best practices when giving example code (the new Integer() and new Boolean() calls are particularly problematic to my eyes beyond other issues).
    – user40980
    Sep 10, 2014 at 18:30

Not the answer you're looking for? Browse other questions tagged or ask your own question.