Is using labeled break a good practice in Java?

Question

I look at some old 2001 code and stumbled upon this statement:

else { do { int c = XMLDocumentFragmentScannerImpl.this.scanContent(); if (c == 60) { XMLDocumentFragmentScannerImpl.this.fEntityScanner.scanChar(); XMLDocumentFragmentScannerImpl.this.setScannerState(1); break label913; } 

I have never seen this before, and found tagged gaps here:

http://docs.oracle.com/javase/tutorial/java/nutsandbolts/branch.html

Doesn't this essentially work like goto ? Is it even good practice to use it? It makes me awkward

Answer 1

No, this is not like goto, because you cannot "go" to another part of the control flow.

On the page you indicated:

The break statement completes the label; It does not transfer control flow to the label. The control flow is transferred to the instruction immediately after the specified (completed) statement.

This means that you can only break the loops that are currently running.

Consider the following example:

 first: for( int i = 0; i < 10; i++) { second: for(int j = 0; j < 5; j ++ ) { break xxx; } } third: for( int a = 0; a < 10; a++) { } 

You can replace xxx with first or second (to break the outer or inner loop), because both loops execute when you press the break statement, but replacing xxx with third will not compile.

Answer 2

This is not as scary as goto , because it only sends control to the end of the labeled statement (usually a loop loop). What makes goto unlikely is that it is an arbitrary branch anywhere, including the labels found above in the method source, so you can have a home loop behavior. The label break functionality in Java does not allow such madness, because control is only moving forward.

I only used it 12 years ago, in a situation where I needed to break out of nested loops, a more structured alternative would make more complex tests in loops. I would not recommend using it a lot, but I would not call it the automatic smell of bad code.

Answer 3

Before reading the rest of this answer, please read. Go to the statement is considered malicious . If you do not want to read it in full, here is what I consider the key point:

The rampant use of the go to statement immediately leads to the fact that it becomes very difficult to find a meaningful set of coordinates that describe the progress of the process.

Or, to rephrase, the problem with goto is that the program can arrive in the middle of a block of code, without the programmer understanding the state of the program at that point. Standard block-oriented constructions are intended for a clear definition of state transitions; a labeled break intended to transfer a program to a certain known state (outside the containing labeled block).

In a real imperative program, the state is not clearly defined by the boundaries of the blocks, so it is doubtful whether labeled break good idea. If a block changes state that is visible from outside the block, and there are several points for exiting the block, then the marked break equivalent to the goto primitive. The only difference is that instead of getting into the middle of a block with an undefined state, you start a new block with an undefined state.

So, in general, I find the labeled break dangerous. In my opinion, this is a sign that the block should be converted to a function with limited access to the enclosing area.

However, this sample code was clearly the product of a parser generator (the OP commented that it was Xerces source code). And parser generators (or code generators in general) often take liberties with the code that they generate, because they have perfect knowledge of the state, and people should not understand it.

Answer 4

You can always replace break new method.

Consider code verification for any common elements in two lists:

 List list1, list2; boolean foundCommonElement = false; for (Object el : list1) { if (list2.contains(el)) { foundCommonElement = true; break; } } 

You can rewrite it like this:

 boolean haveCommonElement(List list1, List list2) { for (Object el : list1) { if (list2.contains(el)) { return true; } } return false; } 

Of course, to check the common elements between two lists, it is better to use list1.retainAll(new HashSet<>(list2)) to do this in O(n) with O(n) extra memory, or to sort both lists in O(n * log n) and then find the common elements in O(n) .

Answer 5

This is not like the goto in which you transfer the flow control back. The label simply shows you (the programmer) where the break occurred. In addition, flow control is carried over to the next statement after break .

On the use of this, I personally think that it does not really matter, because as soon as you start writing code on thousands of lines, it becomes immaterial. But then again, it will depend on the use case.

Answer 6

A cleaner way of expressing intent can be, at least in my opinion, to put a piece of code containing a loop in a separate method and simply return from it.

For example, change this:

 someLabel: for (int j = 0; j < 5; j++) { // do something if ... break someLabel; } 

in it:

 private void Foo() { for (int j = 0; j < 5; j++) { // do something if ... return; } } 

It is also more idiomatic for developers who speak other languages ​​that may work with your code in the future (or in the future of you ).