Evilness of "break" and "continue"



  •  Ok so I'm at this training session, the guy shows a piece of code with a break, and the guy next to me goes... "isn't the use of break considered a bad programming practice?"

    My first reaction (and i said it): "aren't you confusing break with goto??"

    And he goes on: "No, no. All my teachers always said it: don't use break or continue. It's a bad practice."

    And some other colleage says "Yeah, yeah. I heard it too!"

     

    So what the hell? Java? C? Not using break or continue? Are these guys insane? Or am *I* insane? Seriously, is break and continue the spawn of Satan? Or are they just useful instructions which have potencial misuses, just like pretty much *everything* else in IT?

    And even so, what sort of evilness could be so great it would make teachers say these things?

     

    (and what the hell is going on with this piece of crap WYSIWYG editor?? Every time I delete one character it "covertly" deletes another! Is this what Ninja programming is about?)



  • @CrazyBomber said:

    Or are they just useful instructions which have potencial misuses, just like pretty much *everything* else in IT?
    Yeah, pretty much.

    @CrazyBomber said:

    And even so, what sort of evilness could be so great it would make teachers say these things?
    Blame it on structured programming.  Long story short, among other things break and continue create multiple exit points, which goes against classic structred programming.  Obviously, they also get regarded as thinly-veiled gotos.

    Though not as universally reviled as goto, it's a common opinion that break and continue are inherently bad.  This is silly, of course, but I suspect a lot of programming teachers parrot this line.  Like anything else, they're a useful tool if used properly, and a potential nightmare when abused.


  • Discourse touched me in a no-no place

    @CrazyBomber said:

    "isn't the use of break considered a bad programming practice?"
    Oh yes, very. Especially when used in (C or C++) switches. Perhaps they're contraindicated as well?



  • @bstorer said:

    the low-down
     

    Where might I peruse your newsletter before ultimately subscribing?



  • Obviously, we are only talking about loops, not the inevitable break in switch statements.

    In loops, I think they should be avoided. A simple if will do the same trick without the surprise.

    Coincidentally, last weekend my coworkers have spend many hours to pin down an error caused by a "next" statement in a perl program ("next" in perl does the same like "continue" in C). The program was like this (I'll use C syntax here because I'm not a perl programmer):

    while (1) {
      check_something();
      if (somecondition) continue;
      do_something();
      sleep(1);
    }

    As you can see, because of the continue statement, the program never reaches the sleep(1) command, so it does busy waiting and clogs the whole machine. And of course check_something() and do_something() are dozens of LOC, so it's not obvious what really happens and why...



  • Not using break or continue forces another if, another pair of accolades, and another indent level, which I often find a worse deal than two newlines to single out the keyword.

    In your example, I would probably use an if as well rather than continue, but most code is not this concise.



  • @dhromed said:

    Not using break or continue forces another if, another pair of accolades, and another indent level, which I often find a worse deal than two newlines to single out the keyword.

     

    Not really, given the capabilities of todays IDEs and editors. Code folding and stuff.

    In your example, I would probably use an if as well rather than continue, but most code is not this concise.

     

    Ironically, that's how the problem was solved, but my coworkers didn't recognize why this helped, thinking that the observed behavior was an inherent problem of next. (They should have read The Pragmatic Programmer: No, next is not broken)



  • @CrazyBomber said:

    (and what the hell is going on with this piece of crap WYSIWYG editor?? Every time I delete one character it "covertly" deletes another! Is this what Ninja programming is about?)
    And now you are trapped in TDWTF Hell forever!!!



  • @ammoQ said:

    but my coworkers didn't recognize why this helped
     

    Okay, I think there are bigger issues here than a little keyword here or there.

    What did they think was the inherent problem?



  •  @dhromed said:

    @ammoQ said:

    but my coworkers didn't recognize why this helped
     

    Okay, I think there are bigger issues here than a little keyword here or there.

    What did they think was the inherent problem?

    Especially considering that in perl the line would have been:

     next if (condition);

    I think avoiding continue/next & break/last are just another way of dumbing down programming to make it easier on noobs.  The problem is that when making complex software, it is nice to have as much expressive power as possible.  continue and break can do a great deal to make the program more expressive and more similar to actual thinking.

     For instance, if you have a list of items, and your algorithm is:  "for each item in the list, starting at the begining do something.  If do something ever fails, then stop iterating".  Then the following code most closely follows that thinking:

    foreach (Item item in items)
    {
      if (do_something(item)) break;
    }



  •  I used to know a guy who thought break, continue, and goto were bad.  I took it upon myself to change his mind by breaking his finger.  Somehow this didn't sway him.  Not one to give up easily, I figured I'd continue breaking bones until he relented.  Ultimately, I didn't convince him of the usefulness of break or continue, although by the end he was strongly in favor of goto, specifically, he wanted to goto the hospital.  One out of three ain't bad.



  • There are far more evils than using break and continue.

    In C++, using proper RAII technique there should be no problem in using these as your objects should be cleaned up the moment you exit scope. Same with returning in the middle of a function/loop.

    It may be worth commenting at the start of the loop that it could terminate "early", i.e. the "while" condition is not the only thing that will cause a break. The important thing is to have clearly written code that is simple to understand. Loops shouldn't be so huge as to not see that it might be broken early.

    For example a simple linear search might be:

     

    template< typename FwdIter, typename  Pred >

    FwdIter find( FwdIter iter, const FwdIter end, Pred pred )

    {

       for( ; iter != end; ++iter )

       {

         if( pred( *iter ) )

           break;

       }

       return iter;

    }

     


    You could also put return iter in place of the break.

     



  •  The aforementioned people probably use strictly one return statement per method and a cascade of ifs instead a linear run of ifs on the same level for an early escape



  • Don't use break often but me and continue are best buds. Use it plenty. Although I do keep to some unwritten rule of only using it for validation/filtering purposes.

     I was wondering about the whole single exit train of thought, would people like that consider throwing exceptions to be bad as well?



  •  Sorry but I disagree, just because something is easier does not make it a target audience for 'noobs',  and please do not break a foreach, it is confusing. Why are we doing a foreach if we are looking for an early exit, use a while instead.




  • @me_again said:

    Why are we doing a foreach if we are looking for an early exit, use a while instead.
    WTF. So I should just make my code take up an extra few lines so I can shove the same loop into a while's structure all because I'm breaking? No thanks.



  • @Lingerance said:

    So I should just make my code take up an extra few lines so I can shove the same loop into a while's structure all because I'm breaking?
     

    You should use a switch and goto.

    When in javascript, you should generate the switch a a string of code in  a for loop, and then eval() it.

    DOH.



  • @me_again said:

     Sorry but I disagree, just because something is easier does not make it a target audience for 'noobs',  and please do not break a foreach, it is confusing. Why are we doing a foreach if we are looking for an early exit, use a while instead.


    With a foreach the enumerator is also disposed in .NET even if you use break, you want to add that code manually to you while loop?



  • I was just wondering about a corollary to this: Is it bad to use [i]end()[/i] in a PHP script for the same sort of purpose one might use [i]break[/i] in a loop? I understand (?) the traditional argument that you should be explicit about the logic you're imposing but it also seems to me that if I have an [i]if[/i] clause that ends with an [i]end()[/i] that the implicit [i]else[/i] for all code following it is pretty obvious. I find scripts easier to follow this way. Should I be rapped on the knuckles for blasphemy or is this no big deal?

    /old dog trying to learn new tricks



  • @hack said:

    I was just wondering about a corollary to this: Is it bad to use end() in a PHP script for the same sort of purpose one might use break in a loop? I understand (?) the traditional argument that you should be explicit about the logic you're imposing but it also seems to me that if I have an if clause that ends with an end() that the implicit else for all code following it is pretty obvious. I find scripts easier to follow this way. Should I be rapped on the knuckles for blasphemy or is this no big deal?

    /old dog trying to learn new tricks

     

    To be honest I don't really understand the situation you are trying to draw, but if you are using end() in a foreach loop to simulate break, then please just use break.

    I personally also try to mess as little as possible with the internal array pointer unless it is needed for performance reasons. It has a chance to become unobvious and in most cases isn't really needed anyway.



  • @stratos said:

    To be honest I don't really understand the situation you are trying to draw
     

    Just plain ending shit early, wherein [end():script :: break:loop].



  • @dhromed said:

    @stratos said:

    To be honest I don't really understand the situation you are trying to draw
     

    Just plain ending shit early, wherein [end():script :: break:loop].

    Yeah, that. For example, an unauthenticated user is accessing a page intended for authenticated users, so I just want to give them the login script and bypass the rest of the script. I have been designing pages that include conditional paths for auth'd and unauth'd users but I'm thinking maybe I can make the page way simpler (avoiding extraneous clauses) by having the unauth'd case do its thing and then bail out.

    if test {
      do stuff
    } 
    else {
      do other stuff
    }
    

    vs.

    if test { 
      do stuff
      end()
    } 
    do other stuff


  •  ummm, end sets the internal pointer of an array to the last element, die() ends the script, if that is what you are refering to.



  • @stratos said:

     ummm, end sets the internal pointer of an array to the last element, die() ends the script, if that is what you are refering to.

    Oops, actually I was thinking of exit. Should have just said die, though. I dunno. I suck. Let's move on.



  • @hack said:

    Oops, actually I was thinking of exit. Should have just said die, though. I dunno. I suck. Let's move on.
    exit and die are the same keyword, just like echo and print are.



  • @Lingerance said:

    @hack said:
    Oops, actually I was thinking of exit. Should have just said die, though. I dunno. I suck. Let's move on.
    exit and die are the same keyword, just like echo and print are.

    pedantic mode on

    Actually, they are not, they just happen to both output strings. With die and exit you are correct though, those are the same.



  • Hey, I can use HTML on this forum. It's been so long...

    ahHhhHHHh....



  • I frequently use break in loops, though I don't use continue nearly so often.

    The reason? Efficiency.

    Consider simplistic scenario in which you have a loop in which every iteration requires you to check for a condition. Now, assume this condition is dependent on a database query that must be re-run for every iteration. When the condition is met, you need to do some processing, but you only need to do this once.

    Without break, the loop must run until completion. Let's assume 100 iterations: with this scenario, you will have 100 database queries every single time your loop runs, whether you need to actually process the results or not.. With break, you can short-circuit the loop. Assuming there is a 1/100 chance that the condition will be satisfied on any given record, over time the loop will average only 50 queries, nearly halving the amount of processing necessary to check the conditions.



  •  I think we should avoid if-then-else too, because that's basically a goto in disguise:

    [code]if (something) {
    blahblah();
    } else {
    halbhalb();
    }[/code]

    So you see, if something is false, you're basically goto-ing to the part after the else.



  • @pbean said:

     I think we should avoid if-then-else too, because that's basically a goto in disguise:
     

    Slippery slope fallacy.

    Did you use it for fun or for real?

    Who can tell.



  • @dhromed said:

    @pbean said:

     I think we should avoid if-then-else too, because that's basically a goto in disguise:
     

    Slippery slope fallacy.

    Did you use it for fun or for real?

    Who can tell.

    Oh sure, he might have used the slippery slope fallacy in jest now, but next thing you know, he'll be using it all the time seriously, until eventually he dies from dehydration because he can't do anything but use the slippery slope fallacy.


  • It's simply a matter of personal preference. If you don't like continue / break, use an if/else construct or a loop termination condition. Anyone who argues that continue / break are evil because they're too hard to understand or anything like that should seriously consider whether they've picked the right profession, because the semantics of continue and break are about as trivial as anything is ever going to get in programming.



  • @ammoQ said:

    while (1) {
    check_something();
    if (somecondition) continue;
    do_something();
    sleep(1);
    }

     But your "if" means that the program will continue looping through, with a condition check each time, even if it's done.  That doesn't make any sense to me.  I know this charity, see, where you can donate cpu cycles that you don't need -- maybe that would be a better use than the if inside the loop.....



    Fixed your lack of quoting. --Ling


  • @Ottercat said:

    But your "if" means that the program will continue looping through, with a condition check each time, even if it's done.

    If what's done? If it's the loop, that loop won't end (unless there's an invisible break). If it's the check then check_something() might change the state, as it was implied to do.
    @Ottercat said:
    That doesn't make any sense to me.

    The loop makes perfect sense to me. I'd have made sure the sleep(1) took place no matter what though.
    @Ottercat said:
    I know this charity, see, where you can donate cpu cycles that you don't need -- maybe that would be a better use than the if inside the loop.....

    WTF are you on about?



  • 	while(true)
    	{
    		if (bind(m_ServerSocket, (SOCKADDR*)&sockaddr, sizeof(sockaddr)) == SOCKET_ERROR)
    		{
    			if (WSAGetLastError() == WSAEADDRINUSE)
    			{
    				sockaddr.sin_port = htons(ntohs(sockaddr.sin_port) + 1);
    				continue;
    			}
    			return WSAGetLastError();
    		}
    		break;
    	}

    Do I win? I have break, continue and return in 1 while loop. With break as last entry in the while as bonus.



  • You win teh internets for screwing with the next guy working on your code

    while(bind(m_ServerSocket, (SOCKADDR*)&sockaddr, sizeof(sockaddr)) == SOCKET_ERROR)
    {
    if (WSAGetLastError() != WSAEADDRINUSE)
    {
    return WSAGetLastError();
    }
    sockaddr.sin_port = htons(ntohs(sockaddr.sin_port) + 1);
    }


  •  Well, I just skimmed through the thread, so somebody else might have pointed it out, but I would prefer

    int a[ ] = {some array of numbers}

    for (i = 0; i < n; ++i) {
    if (a[i] == some element I'm searching for )
    break;
    }

    if (i < n) {
    do_stuff();
    }

    over

    int found = false;
    for (i = 0; !found && i < n; ++i) {
    if (a[i] == some element I'm searching for )
    found = true;
    }

    if (found) {
    do_stuff();
    }

    but that might just be me.



  • @enfiskutensykkel said:

     Well, I just skimmed through the thread, so somebody else might have pointed it out, but I would prefer

    int a[ ] = {some array of numbers}

    for (i = 0; i < n; ++i) {
    if (a[i] == some element I'm searching for )
    break;
    }

    if (i < n) {
    do_stuff();
    }

    over

    int found = false;
    for (i = 0; !found && i < n; ++i) {
    if (a[i] == some element I'm searching for )
    found = true;
    }

    if (found) {
    do_stuff();
    }

    but that might just be me.

     

    I prefer PHP awesomeness
    a = array(); //some array of numbers
    if (in_array($someElmentImSearchFor, $a))
    {
      do_stuff();
    }
    


  • @enfiskutensykkel said:

     Well, I just skimmed through the thread, so somebody else might have pointed it out, but I would prefer

    int a[ ] = {some array of numbers}

    for (i = 0; i < n; ++i) {
    if (a[i] == some element I'm searching for )
    break;
    }

    if (i < n) {
    do_stuff();
    }

    over

    int found = false;
    for (i = 0; !found && i < n; ++i) {
    if (a[i] == some element I'm searching for )
    found = true;
    }

    if (found) {
    do_stuff();
    }

    but that might just be me.

     

     I use this:

    bool playerIsAlive = false;
    for (int i = 0; i < game.Players.Length; i++)
    if (game.Players[i].Health > 0)
    {
    playerIsAlive = true;
    break;
    }

    This is what I did once to check if any player is alive. If no player is alive, play a cutscene/end game.With this code, if he is playing 2 player or 3 player or whatever, I don't have to change the code.

     



  • Surprised nobody has mentioned LINQ's syntactic sugar ;)

    int[ ] values = new [ ] { ... };
    int[ ] searchValues = new [ ] { ... };
    
    var result = from v in values
                     where searchValues.Contains(v)
                      select do_Something(v);
    


  • @RaspenJho said:

    Surprised nobody has mentioned LINQ's syntactic sugar ;)
    int[ ] values = new [ ] { ... };
    int[ ] searchValues = new [ ] { ... };
    

    var result = from v in values
    where searchValues.Contains(v)
    select do_Something(v);

    var values =  { ... };
    var searchValues =  { ... };

    var result = from v in values
                     where searchValues.Contains(v)
                      select do_Something(v);

    Even shorter syntactic sugar.... :)



  • "continue" and "break" are nothing more than a pleasant syntax for a "goto". Apparently by giving them cute names and restricting their usages to particular control structures, they no longer draw the ire of the "all gotos are all bad all the time" crowd.

    If what you want to do is a continue-to-outer, you could simply define a label at the top of the outer loop and then "goto" that label. If you felt that doing so did not impede the comprehensibility of the code, then that might be the most expedient solution.

    However, I would take this as an opportunity to consider whether your control flow would benefit from some refactoring. Whenever I have conditional "break" and "continue" in nested loops, I consider refactoring.

    Consider:

    successfulCandidate = null;
    foreach(var candidate in candidates)
    {
     
    foreach(var criterion in criteria)
     
    {
       
    if (!candidate.Meets(criteria))
       
    {  // TODO: no point in continuing checking criteria.
           
    // TODO: Somehow "continue" outer loop to check next candidate
       
    }
     
    }
      successfulCandidate
    = candidate;
     
    break;
    }
    if (successfulCandidate != null) // do something

    Two refactoring techniques:

    First, extract the inner loop to a method:

    foreach(var candidate in candidates)
    {
     
    if (MeetsCriteria(candidate, criteria))
     
    {
          successfulCandidate
    = candidate;
         
    break;
     
    }
    }

    Second, can all the loops be eliminated? If you are looping because you are trying to search for something, then refactor it into a query.

    var results = from candidate in candidates 
                  where criteria
    .All(criterion=>candidate.Meets(criterion))
                 
    select candidate;
    var successfulCandidate = results.FirstOrDefault();
    if (successfulCandidate != null)
    {
     
    do something with the candidate
    }

    If there are no loops then there is no need to break or continue!



  • It's the people who oppose break, continue AND goto who are insane. (And I believe it's largely the same crowd who advocate only one return per function or method, too.)

    While I do agree that for educational purposes, it's good that they are avoided for a good while UNTIL the student thoroughly understands why they needed to avoid them. But as things stand, I do feel that most people take the principle too far, and in fact I believe the need for try-catch(-finally) largely comes from trying to avoid goto at all costs.

    There is no try-catch in C, so we end up seeing a lot of code like this:

    ret=SUCCESS;
    if(init_stuff()) {
    if(a=get_a(...)) {
    if(b=get_b(...)) {
    if(c=get_c(...)) {
    if(!do(a,b,c)) {
    ret=FAILED;
    log("error doing a,b,c");
    }
    } else {
    ret=FAILED;
    log("error getting c");
    }
    cleanup(c);
    } else {
    ret=FAILED;
    log("error getting b");
    }
    cleanup(b);
    } else {
    ret=FAILED;
    log("error getting a");
    }
    cleanup(a);
    cleanup_stuff();
    } else
    ret=FAILED;
    return ret;

    And not only do people write functions like that, they'll actually go to great lengths to defend that it is the appropriate way to do it! 

    If I were to rewrite the above like this:

    error=NO_ERROR;
    if(!init_stuff())
    return FAILED;

    if(!(a=get_a(...))) { error=ERROR_A; goto cleanup_a; }
    if(!(b=get_b(...))) { error=ERROR_B; goto cleanup_b; }
    if(!(c=get_c(...))) { error=ERROR_C; goto cleanup_c; }

    if(!do(a,b,c)) { error=ERROR_DO_ABC; }

    cleanup_c: cleanup(c);
    cleanup_b: cleanup(b);
    cleanup_a: cleanup(a);

    cleanup_stuff();

    if(error!=NO_ERROR) {
    log(error_msgs[error]);
    return FAILED;
    }
    return SUCCESS;

    A lot of people would protest the "bad practices" I'm using (although it works almost exactly like try-catch). I'd also probably be let known how readability suffers because the code has not been properly indented. 



  •  I am learning it right in school, but does it means that we are not in a right track, which the teachers discusses as opposite to this.



  • ANSI Common Lisp has LOOP:

    (LOOP FOR v IN values
          IF (MEMBER v searchvalues)
          RETURN (do_something v))
    

    Or, if you like ITERATE better:

    (ITER (FOR v IN values)
          (IF (MEMBER v searchvalues)
            (RETURN (do_something v))))
    

    Or even, for this specific example,

    (LET ((v (INTERSECTION values searchvalues)))
      (IF v
        (do_something v)))
    

    Or using alexandria:

    (AIF (INTERSECTION values searchvalues)
      (do_something IT))
    

    (all just written, not tested)



  • @flop said:

    ANSI Common Lisp has LOOP:
     

    Ah, and what I forgot to say what that internally, most implementations just translate such macros (LOOP, ITERATE, DOTIMES, ...) into a (TAGBODY ...) - which uses GO again ...

    (MACROEXPAND                                                              
      '(ITER (FOR v IN values)                                                
             (IF (MEMBER v searchvalues)                                      
                 (RETURN (do_something v)))))                                 
    

    (LET* ((#:LIST1 NIL) (V NIL))
    (BLOCK NIL
    (TAGBODY
    (PROGN (SETQ #:LIST1 VALUES))
    LOOP-TOP-NIL
    (PROGN
    (IF (ENDP #:LIST1)
    (GO LOOP-END-NIL))
    (SETQ V (CAR #:LIST1))
    (SETQ #:LIST1 (CDR #:LIST1))
    (IF (MEMBER V SEARCHVALUES)
    (RETURN-FROM NIL (DO_SOMETHING V))))
    (PROGN)
    (GO LOOP-TOP-NIL)
    LOOP-END-NIL
    (PROGN))
    NIL))



  • @PJH said:

    Oh yes, very. Especially when used in (C or C++) switches. Perhaps they're contraindicated as well?
    Mhm, break is a satanic word to mumble inside a switch block. I (dis-)honestly see the superior beauty of goto in such a case - or even out of such a case, in C/C++. ;-)



  • On the other hand, those preferring goto over break might be accused of switchcraft.



  • @spezialpfusch said:

    switchcraft

    You've waited for years to make that joke. Finally!


  • Discourse touched me in a no-no place

    @The Flaming Foobar said:

    It's the people who oppose break, continue AND goto who are insane. (And I believe it's largely the same crowd who advocate only one return per function or method, too.)

    While I do agree that for educational purposes, it's good that they are avoided for a good while UNTIL the student thoroughly understands why they needed to avoid them. But as things stand, I do feel that most people take the principle too far, and in fact I believe the need for try-catch(-finally) largely comes from trying to avoid goto at all costs.

    There is no try-catch in C, so we end up seeing a lot of code like this:

    [...]

    And not only do people write functions like that, they'll actually go to great lengths to defend that it is the appropriate way to do it!

    If I were to rewrite the above like this:

    [...]

    A lot of people would protest the "bad practices" I'm using (although it works almost exactly like try-catch). I'd also probably be let known how readability suffers because the code has not been properly indented. 

    You wait until you encounter code where they're using this sort of nastiness (plus loops!) so much that their code indent goes over 80 columns even with a per-level-indent-width of 2. 40 levels of indent and all with complex flag variables used to encode the bits of control structures that didn't go neatly within their scheme.

    When such things are encountered, much refactoring is required. The code needs to become much less deeply indented (and real control structures used for control rather than sharing all that with a smattering of other variables) and the creator of said code needs to be refactored out of any contact with the code too. You know it makes sense. (Hint: encourage them to go and work for your greatest competitor.)


Log in to reply