views:

1805

answers:

19

Possible Duplicate:
Break statements In the real world

Hi,

Is it a bad practice to use break statement inside a for loop?

Say, I am searching for an value in an array. Compare inside a for loop and when value is found, break; to exit the for loop.

Is this a bad practice? I have seen the alternative used : define a variable vFound and set it to true when the value is found and check vFound in the for statement condition. But is it necessary to create a new variable just for this purpose?

I am asking in the context of a normal C or C++ for loop.

P.S: The MISRA coding guidelines advise against using break;

-Kiki

+23  A: 

Using break as well as continue in a for loop is perfectly fine.

It simplifies the code and improves its readability.

Developer Art
thank you, DeveloperArt :)
kiki
Yes.. For a time I didn't like the idea and coded around it, but it didn't take long before I realized the... "workaround" was often a maintenance nightmare. Well, not a nightmare, but more error-prone.
MetalMikester
+9  A: 

It depends on the language. While you can possibly check a boolean variable here:

for (int i = 0; i < 100 && stayInLoop; i++) { ... }

it is not possible to do it when itering over an array:

for element in bigList: ...

Anyway, break would make both codes more readable.

eumiro
thank you, eumiro.
kiki
Trillian
@Trillian - you're right, this wanted to look like C ;-)
eumiro
+2  A: 

In your example you do not know the number of iterations for the for loop. Why not use while loop instead, which allows the number of iterations to be indeterminate at the beginning?

It is hence not necessary to use break statemement in general, as the loop can be better stated as a while loop.

Schedler
A "for" loop is often good if there's a known maximum number of iterations. That having been said, a while(1) loop is sometimes better in a scenario where one is searching for an item and plans to create it if it doesn't exist. In that case, if the code finds the item it does a direct break, and if it hits the end of the array it creates a new item and then does a break.
supercat
In the example given -- searching through an array for a key, the for loop is the proper idiomatic construct. You don't use a while loop to walk through an array. You use a for loop. (or a for-each loop if available). You do this as a courtesy to other programmers, as they recognize the "for (int i=0; i < ra.length; i++){}" construct immediately as "Walk through the array" Adding a break, to this construct is simply a "Quit early if you can" statement.
chris
+54  A: 

No, break is the correct solution.

Adding a boolean variable makes the code harder to read and adds a potential source of errors.

smirkingman
thnak you, smirkingman :)
kiki
agreed. Especially if you are looking to exit the loop under more than a single condition. The boolean to leave a loop can get really confusing then.
Rontologist
+30  A: 

You can find all sorts of professional code with 'break' statements in them. It perfectly make sense to use this whenever necessary. In your case this option is better than creating a separate variable just for the purpose of coming out of the loop.

Amit S
edited: "professional code", not "professional codes"
unforgiven3
A: 

Ofcourse, break; is the solution to stop the for loop or foreach loop. I used it in php in foreach and for loop and found working.

lakum4stackof
A: 

Depends on your use case. There are applications where the runtime of a for loop needs to be constant (e.g. to satisfy some timing constraints, or to hide your data internals from timing based attacks).

In those cases it will even make sense to set a flag and only check the flag value AFTER all the for loop iterations have actually run. Of course, all the for loop iterations need to run code that still takes about the same time.

If you do not care about the run time... use break; and continue; to make the code easier to read.

ndim
A: 

On MISRA 98 rules, that is used on my company in C dev, break statement shall not be used...

Edit : Break is allowed in MISRA '04

Benoît
Exactly why I asked this question! I find no good reason why such a rule is there as a coding guideline. Also, as everyone else here said, break; looks better.
kiki
I don't know exactly why it is forbidden (more clever person than me have thinking about it...) but break (and multiple return) are better for readability (in my opinion, but my opinions are not corporates rules...)
Benoît
luis.espinal
We used MISRA 98 rules... It is exact that MISRA 2004 changes the rules
Benoît
+4  A: 

break is a completely acceptable statement to use (so is continue, btw). It's all about code readability -- as long as you don't have overcomplicated loops and such, it's fine.

It's not like they were the same league as goto. :)

riviera
I have seen it argued that a break statement *is* effectively a **goto**.
glenatron
The problem with goto is that you can point it anywhere. Both break and continue preserve the modularity of the code. That argument is on the same level as having a single exit point for each function.
Zachary Yates
I've heard it argued that having multiple exit points for a function *is* effectively a **goto**. ;D
glenatron
@glenatron The problem with goto isn't the goto statement, it is the introduction of the label that the goto jumps to that is the problem. Whem you read a goto statement, there is no uncertainty about control flow. When you read a label, there is lots of uncertainty about control flow (where are all the gotos that transfer control to this label?). A break statement doesn't introduce a label, so it doesn't introduce any new complications.
Stephen C. Steel
I quite agree. The first comment was mostly because I have seen that argument and it made me smile, then the second comment because I have also seen that argument and it was a great setup. As everyone knows, the correct way to handle these kind of control flow matters is to throw an exception and allow it to be caught in some byzantine exception handling system. Much less confusing. ;p
glenatron
The "break" is a specialzied goto that can only leave blocks. The historical problems with "goto" were (1) it could be, and often was, used to construct overlapping loop blocks in a way that isn't possible with proper control structures; (2) a common way to do an "if-then" construct which was usually "false" was to have the true case branch off to an unrelated place in the code, and then branch back. This would cost two branches in the rare case, and zero in the common case, but it made the code look hideous.
supercat
+5  A: 

I agree with others who recommend using break. The obvious consequential question is why would anyone recommend otherwise? Well... when you use break, you skip the rest of the code in the block, and the remaining iterations. Sometimes this causes bugs, for example:

  • a resource acquired at the top of the block may be released at the bottom (this is true even for blocks inside for loops), but that release step may be accidentally skipped when a "premature" exit it caused by a break statement (in modern C++, "RAII" is used to handle this in an even better, exception-safe way: basically, object destructors free resources reliably no matter how a scope is exited)

  • someone may change the conditional test in the for statement without noticing that there are other delocalised exit conditions

  • ndim's answer observes that some people may avoid breaks to maintain a relatively consistent loop run-time, but you were comparing break against use of a boolean early-exit control variable where that doesn't hold

Every now and then people observing such bugs realise they can be prevented/mitigated by this "no breaks" rule... indeed, there's a whole related strategy for "safer" programming called "structured programming", where each function is supposed to have a single entry and exit point too (i.e. no goto, no early return). It may eliminate some bugs, but it doubtless introduces others. Why do they do it?

  • they have a development framework that encourages a particular style of programming / code, and they've statistical evidence that this produces a net benefit in that limited framework, or
  • they've been influenced by programming guidelines or experience within such a framework, or
  • they're just dictatorial idiots, or
  • any of the above + historical inertia (relevant in that the justifications are more applicable to C than modern C++).
Tony
+2  A: 

It's perfectly valid to use break - as others have pointed out, it's nowhere in the same league as goto.

Although you might want to use the vFound variable when you want to check outside the loop whether the value was found in the array. Also from a maintainability point of view, having a common flag signalling the exit criteria might be useful.

Ace
+6  A: 

Far from bad practice, Python (and other languages?) extended the for loop structure so part of it will only be executed if the loop doesn't break.

for n in range(5):
    for m in range(3):
        if m >= n:
            print 'stop!'
            break
        print m,
    else:
        print 'finished.'

Output:

stop!
0 stop!
0 1 stop!
0 1 2 finished.
0 1 2 finished.

Equivalent code without break and that handy else:

for n in range(5):
    aborted = False
    for m in range(3):
        if not aborted:
            if m >= n:
                print 'stop!'
                aborted = True
            else:            
                print m,
    if not aborted:
        print 'finished.'
Nick T
Oooh, I like that and have sometimes wished for it in C. Nice syntax too, which could be adapted into a future C-like language without ambiguity.
supercat
+2  A: 

In the embedded world, there is a lot of code out there that uses the following construct:

    while(1)
    { 
         if (RCIF)
           gx();
         if (command_received == command_we_are_waiting_on)
           break;
         else if ((num_attempts > MAX_ATTEMPTS) || (TickGet() - BaseTick > MAX_TIMEOUT))
           return ERROR;
         num_attempts++;
    }
    if (call_some_bool_returning_function())
      return TRUE;
    else
      return FALSE;

This is a very generic example, lots of things are happening behind the curtain, interrupts in particular. Don't use this as boilerplate code, I'm just trying to illustrate an example.

My personal opinion is that there is nothing wrong with writing a loop in this manner as long as appropriate care is taken to prevent remaining in the loop indefinitely.

Nate
Could you elaborate on that? It looks to me as if the loop does absolutely nothing... unless there are `continue` statements inside the application logic. Are there?
Rene Saarsoo
continue statements are unnecessary as you are already bound by an infinite loop. You basically perform the necessary logic required to achieve a goal and if that goal is not met in n number of attempts or a timeout condition expires, you exit the loop via a break construct and take corrective action or inform the calling code of an error. I see this type of code often in uCs that communicate via a common bus (RS-485, etc.) You basically attempt to seize the line and communicate without collisions, if collisions happen, you wait a duration determined by a random number and try again.
Nate
Conversely, if things go well, you also use the break statement to exit once the condition has been met. Given this case, the code following the loop is executed and everyone is happy.
Nate
+3  A: 

General rule: If following a rule requires you to do something more awkward and difficult to read then breaking the rule, then break the rule.

In the case of looping until you find something, you run into the problem of distinguishing found versus not found when you get out. That is:

for (int x=0;x<fooCount;++x)
{
  Foo foo=getFooSomehow(x);
  if (foo.bar==42)
    break;
}
// So when we get here, did we find one, or did we fall out the bottom?

So okay, you can set a flag, or initialize a "found" value to null. But

That's why in general I prefer to push my searches into functions:

Foo findFoo(int wantBar)
{
  for (int x=0;x<fooCount;++x)
  {
    Foo foo=getFooSomehow(x);
    if (foo.bar==wantBar)
      return foo;
  }
  // Not found
  return null;
}

This also helps to unclutter the code. In the main line, "find" becomes a single statement, and when the conditions are complex, they're only written once.

Jay
Exactly what I wanted to say. Often when I find myself using `break`, I try to find some way to refactor the code into a function, so that I can use `return` instead.
Rene Saarsoo
A: 

I don't see any reason why it would be a bad practice PROVIDED that you want to complete STOP processing at that point.

Cornelius J. van Dyk
+1  A: 

I disagree!

Why would you ignore the built-in functionality of a for loop to make your own? You do not need to reinvent the wheel.

I think it makes more sense to have your checks at the top of your for loop like so

for(int i = 0; i < myCollection.Length && myCollection[i].SomeValue != "Break Condition"; i++)
{
//loop body
}

or if you need to process the row first

for(int i = 0; i < myCollection.Length && (i == 0 ? true : myCollection[i-1].SomeValue != "Break Condition"); i++)
{
//loop body
}

This way you can write a function to perform everything and make much cleaner code.

for(int i = 0; i < myCollection.Length && (i == 0 ? true : myCollection[i-1].SomeValue != "Break Condition"); i++)
{
    DoAllThatCrazyStuff(myCollection[i]);
}

Or if your condition is complicated you can move that code out too!

for(int i = 0; i < myCollection.Length && BreakFunctionCheck(i, myCollection); i++)
{
    DoAllThatCrazyStuff(myCollection[i]);
}

"Professional Code" that is riddled with breaks doesn't really sound like professional code to me. It sounds like lazy coding ;)

Biff MaGriff
lazy coding IS professional code :)
Jason
Only if it isn't a pain in the butt to maintain :)
Biff MaGriff
I tend to practice "GTFO ASAP". That is, if I can exit a structure cleanly then I should do so as soon as possible, and as near to the condition that specifies exit.
chris
Well that works too. I think the argument I'm trying to get across is that if you are using a for loop it doesn't hurt to use its built in functionality to make your code readable and modular. If you absolutely need to have break statements inside of your loop I would sit and have a think about why that level of complexity is needed.
Biff MaGriff
+1  A: 

There is nothing inherently wrong with using a break statement but nested loops can get confusing. To improve readability many languages (at least Java does) support breaking to labels which will greatly improve readability.

int[] iArray = new int[]{0,1,2,3,4,5,6,7,8,9};
int[] jArray = new int[]{0,1,2,3,4,5,6,7,8,9};

// label for i loop
iLoop: for (int i = 0; i < iArray.length; i++) {

    // label for j loop
    jLoop: for (int j = 0; j < jArray.length; j++) {

        if(iArray[i] < jArray[j]){
            // break i and j loops
            break iLoop;
        } else if (iArray[i] > jArray[j]){  
            // breaks only j loop
            break jLoop;
        } else {
            // unclear which loop is ending
            // (breaks only the j loop)
            break;
        }
    }
}

I will say that break (and return) statements often increase cyclomatic complexity which makes it harder to prove code is doing the correct thing in all cases.

If you're considering using a break while iterating over a sequence for some particular item, you might want to reconsider the data structure used to hold your data. Using something like a Set or Map may provide better results.

cyber-monk
+7  A: 

Lots of answers here, but I haven't seen this mentioned yet:

Most of the "dangers" associated with using break or continue in a for loop are negated if you write tidy, easily-readable loops. If the body of your loop spans several screen lengths and has multiple nested sub-blocks, yes, you could easily forget that some code won't be executed after the break. If, however, the loop is short and to the point, the purpose of the break statement should be obvious.

If a loop is getting too big, use one or more well-named function calls within the loop instead. The only real reason to avoid doing so is for processing bottlenecks.

e.James
A: 

I did some analysis on the codebase I'm currently working on (40,000 lines of JavaScript).

I found only 22 break statements, of those:

  • 19 were used inside switch statements (we only have 3 switch statements in total!).
  • 2 were used inside for loops - a code that I immediately classified as to be refactored into separate functions and replaced with return statement.
  • As for the final break inside while loop... I ran git blame to see who wrote this crap!

So according to my statistics: If break is used outside of switch, it is a code smell.

I also searched for continue statements. Found none.

Rene Saarsoo