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

  • 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.

    Doesn't take much to forget which one does what.

    No. Neither is goto. Knowing when to use them is the key. They are tools in the toolbox. You use them when they provide clear & succinct code.

    That type of coding can lead to what I like to refer to as "code ramble." Code ramble is what occurs when one tries to include the functionality of multiple functions within one function. Code ramble is almost a given in Java because the language allows for and its libraries promote the intermixing of code and data declarations in methods. One must be very vigilant to avoid coding several hundred line, poorly-factored methods. My upper bound for a method to sixty printed lines. That count includes blank lines.

    I cannot voice my support for this style of coding strongly enough. Multiple levels of nested conditionals are so much worse than this approach. I'm usually not militant about coding style, but this is almost a deal-breaker for me.

    Obviously your boss doesn't write (enough) code. If he did, he would know that all keywords (yes, even `goto`) are useful in **some** cases.

    He didn't call them useless... just called them bad practice

    I would add to Klaim's answer: ideally (and particularly if they get long) your preconditions should be in a local function of its own, so then you do not mix break and continue :)

    Your question seems to suggest that `break` may/maynot be bad practice compared with `return`. Those `return` statements are alternative exit-points for the loop too - and also alternative exit-points for the whole function. There is a time and a place, of course, but the thing to watch out for is having non-obvious exit points (a weaker but pragmatic version of the single-exit-point rule). Personally, I'd be nervous about mixing `continue` in with a mess of `break` exit points, though - the fact that one or two of the exit-points behave differently isn't necessarily obvious at a glance.

    @Thorbjørn - I never thought about `continue` being a sensible keyword for `continue after the end of the loop` until I tried to write a comment calling you an idiot. Damn! That said, I don't see how `break` can be misinterpreted as `go back to the top of the loop`.

    @Thorbjørn - Continue = skip to the next iteration of the loop, break = stop the loop... right?

    just because bad programmers use it, doesn't mean there can be no legitimate uses as well :)

    @ThomasX no need to be rude. I wondered - have you tried maintaining such code written by _others_ yet?

    @Thorbjørn - I have, and rarely had a problem with it, but I've mostly (not always) had to maintain code where those were used sensibly. One thing that sticks out in memory is that for a long time I ignored and forgot about `continue` (there's no such thing in Pascal) - until I found it in someone elses code that I was maintaining. I remember it because the confusion was momentary, and because it was an "I'm such an idiot - why haven't I been doing this myself?" moment.

    _Bad programmers use break and continue_ doesn't mean that good programmers don't. Bad programmers use if and while as well.

    I would definately change your second example to include the loop_count and time_exect to be inside the where-clause instead of having their own break.

    @PieterB, what where-clause?

    @Mikhail sorry, I meant having them in the "while" condition, not where. while (primary_condition && (loop_count > 1000)&& time_exect > 3600) ) {

    Bad programmers follow rules blindly. Worse programmers randomly either follow or don't follow rules :-( Good programmers know when to follow rules and when to ignore them. Some rules are good to prevent bad programmers from writing even worse code and are pointless to apply to good programmers.

    The break-conditions can go together in the while-condition, the continue-conditions can be inversed into proper nested ifs that will only execute if the condition is right. I tend to only execute code in situations that can be trusted and that is INSIDE an if-statement where you just evaluated the proper conditions. However, if you only write some breaks or continues at the top, *who knows* you might have forgotten about some extreme situation where your code still will end up because you didn't include a break or continue for that specific situation! My advice: only execute when sure.

    Good programmers go to single-exit. Bad programmers goto everywhere.

    @klaar Inside a loop, putting everything inside an `if(condition){}` is 100% the same as having `if(!condition) continue;` at the beginning. You're still running the code "where you just evaluated the proper conditions". Forgetting an edge case will happen just the same way for both ways. However, the latter saves you up to 2 lines and 1 indent.

    Yes, I was wrong about that. Although having my code wrapped in an `if` or `else` statement gives me a visual reminder that this code is executed conditionally, while in _your_ code you have to keep that fact in mind, but if you don't and you forget, then you're entering the Danger Zone.

    Either case, it's silly to have to put so many condition checks in your code. It's far better to delegate that and have a single "should I or should I not" condition to which that combination boils down.

    If your boss says something like "bad programmers" it is a good reason to change the boss. We call such people "sofa experts".

  • Klaim

    Klaim Correct answer

    10 years ago

    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.

    Personally, I'd prefer an if-statement to handle pre-conditions. `if(dataIsOK) { someSomething(data) }` is clearer, IMO.

    For me it depends on how much conditions and different behaviours on each condition you have to check. If there is one condition, one behaviour, yes it's easier to just imbricate in a if. If however you have several conditions with return or breaks or continue or other special behaviours, I prefer short and clear statements (like assertions but in a non-error way).

    @FrustratedWithFormsDesigner: if-statements are limited when you want to perform some action like logging, using breaks you can log more detailed ( if(!conditionA){ log("Condition A failed"); break;} ).

    @Klaim: It can be argued that any routine that has several exit points is a poorly-factored routine. A properly factored routine should do one thing and one thing only.

    @bit-twiddler: I agree on the principle, but the purpose of a routine isn't directly related to it's behaviour. Doing one thing might result in different results, that might, or not, have been generated by different instruction flows. As far as it is correct and, indeed, does only one thing, and does it well, then the way it's written inside is a totally orthogonal problem. I believe that's why we like to enforce abstraction through encapsulation (hidding the guts of the process), polymorphism (hidding wich implemenation will be called), generic programming and other abstraction techniques.

    @Klaim: I have been writing code for over thirty years and have never used the continue statement or multiple exit points in my code. These language features are frankly not needed in properly structure logic. The only time that I use break is in case statements. Furthermore, multiple results can be handled by assigning a result to a local "result" variable and returning that variable at the end of the routine. One should not code for oneself. One should code for the poor sole who is going to have maintain the code in the future without the benefit of the original design rational.

    @bit-twiddler: I agree that we could code the same algorithms without those features. I don't agree on the unusefulness (is it correct english) of having some shortcuts to do the same (clearly return, continue and break are just shortcuts). I believe syntactically permissive languages are more prone to allow those shortcuts because they rely a lot on the programmer's expertise/experience to use them where it's reasonnable. I also believe that not providing them is not a real problem.

    @bit-twiddler: this is a very C-ish mindset. A temporary variable can be modified later on, so a single typo 20 lines down from here could erase that carefully crafted result. An immediate return (or break, or continue) however is extremely clear: I can stop reading **now** because I know it can't possibly be modified *further down*. It's good for my little brain, really makes trudging through the code easier.

    @Matthieu I agree. Exit a block when you receive a result that satisfies the block's purpose.

    @bit-twiddler - the single-exit-point principle is separate from the single-responsibility-principle. I don't agree that checking is always separate from acting WRT the single responsibility. In fact "single responsibility" always strikes me as a subjective term. For example, in a quadratic-solver, should calculating the discriminant be a separate responsibility? Or can the whole quadratic formula be a single responsibility? I'd argue that it depends on whether you have a separate use for the discriminant - otherwise, treating it as a separate responsibility is probably excessive.

    -1. If the code is clear and concise, no `break`/`continue` statement will act as a hidden trap. Calling any code bad without seeing it first is ignorant.

    I'm afraid I'm gonig to have to agree with the others. Well structured code can have a break other then at the top. For instance what if you can't do your error checking immediately? You have to fist derive foo before you can check rather foo is valid to decide rather to continue/break. it can still be clear if you check if foo is your desired value and break as soon as foo is calculated, even if that is in the middle of your code

    That answer is a rule of thumb, not a hard rule. It works in most cases, feel free to break it if it make sense in your context.

    I use them as often as possible. There is beauty in their aggressive GTFO, no hidden meaning, no possibility that your control variable lies. Moreover, the use of this programming style keeps the code very flat, making easy to spot these statements. If you write short functions (and you should), you will never run into trouble.

License under CC-BY-SA with attribution


Content dated before 6/26/2020 9:53 AM