if (condition) continue; OR if (!condition) { ... }? (style preference)

Question

I know this is a matter of style, hence the subjective tag. I have a small piece of code, with two nested conditions. I could code it in two ways, and I'd like to see how more experienced developers think it should look like.

Style 1:

while (!String.IsNullOrEmpty(msg = reader.readMsg()))
{
    RaiseMessageReceived();
    if (parseMsg)
    {
        ParsedMsg parsedMsg = parser.parseMsg(msg);
        RaiseMessageParsed();
        if (processMsg)
        {
            process(parsedMsg);
            RaiseMessageProcessed();
        }
    }
}

Style 2:

while (!String.IsNullOrEmpty(msg = reader.readMsg()))
{
    RaiseMessageReceived();
    if (!parseMsg) continue;

    ParsedMsg parsedMsg = parser.parseMsg(msg);
    RaiseMessageParsed();
    if (!processMsg) continue;

    process(parsedMsg);
    RaiseMessageProcessed();
}

(Side question: how do I put empty lines in the source code sample?)

Answer 1

I prefer Style 1 - with the indenting.

Answer 2

I prefer Style 2 - with the continue statement.

Answer 3

I definitely prefer the first version. The continue statement is very nice when not overused.

I'd treat this along the same lines as multiple return statements. They are good for guard clauses and have good usefulness when clarity is improved, but shouldn't be overused.

Also, two spaces on a line should insert a line break for you in code blocks.

Answer 4

I find continue statements make the code harder to follow (and hence debug). There are cases where you might use them anyway, but I don't think your example is one of those cases.

Answer 5

Avoid continue statements where possible.

We avoid goto as much as possible, don't we - doesn't it also make sense to avoid it's cousin continue?

Answer 6

I prefer style 1 with the indenting, it is cleaner, and easier to understand just by looking at it I can see the layout of the code.

Answer 7

In the example shown, I'd go with style 1. If my method were big enough that the nesting became a problem (and there wasn't a better way to refactor the code), then I'd consider style 2. But for just the two compact cases shown, definitely style 1.

Answer 8

I prefer Style2. Moreover same sample is described at Refactoring.com Replace Nested Conditions

Answer 9

Style 1 is simply CLEARER, IMHO. Nothing against Continue per se, but as people have said earlier, the indentation makes for easier following.

Answer 10

The first style is certainly clearer -- it says what you (apparently) mean, ie, get a message, and if the parseMsg flag is set, then try to parse it; if that's successful, process it. On the other hand, the second version is going to process significantly fewer instructions, especially if parseMsg is set. I'd be tempted to go halfway between the two:

while (!String.IsNullOrEmpty(msg = reader.readMsg())){
    RaiseMessageReceived();

    if (!parseMsg) continue ;

    ParsedMsg parsedMsg = parser.parseMsg(msg);
    RaiseMessageParsed();
    if (processMsg){
       process(parsedMsg);
       RaiseMessageProcessed();
    }
}

...on the theory that you're skipping all the parsing in the special case of parsing being turned off.

Answer 11

I prefer style 1 and equate the continue statement to a goto statement. I admit that it might not be as efficient during execution as the other, but I find that code comprehension efficency almost always is more important than code execution efficency.

Answer 12

Both of these are bogus. Do not put assignment in conditional expressions.

(!String.IsNullOrEmpty(msg = reader.readMsg()))

You only do this because of the querky behavior of the reader - why does the reader give you a non-message to indicate that reading is done? Here's an alternative with a better designed reader:

while (reader.HasMessage())
{
  string msg = reader.GetMessage();
  HandleMessage(msg);
}

Answer 13

In principle I agree with the majority who prefer style 1. This is what Steve Mcconnell endorses in "Code Complete" - say what you mean, i.e. if you are more interested in the condition being true, while the false state is rarer or not preferred, then state the preferred version.

In practice though I often find myself using style 2, because I like to weed out all the possible error / invalid states first. After I get rid of all the possibilities I am not interested in, I can write the heart-of-the-matter code down to the end of the routine without constantly wondering if I need to guard against some condition or other. Basically, the attitude is, get rid of the chaff, then do the real work in peace.

Answer 14

I personally prefer style 2 for a number of reasons;

• You can hit a potention bug where you indent after the if but forget the braces.

• You have less risk of hitting a floating else problem, i.e. with multiple nested ifs you can get confused with which if a given else belongs.

• You avoid excessive indenting that can lead to code going off the page

When I am coding this, I tend to put the continue on a seperate line;

if (!parseMsg) 
    continue;

For the reasons it makes it more visible, and it is easier to assign a break point to it.

Answer 15

Before worrying about cosmetic irrelevances, I would remove the use of global variables to communicate between functions. Presumably your language supports return values from functions. Use these to either

• express success or failure of each step, or
• have each transition function return the next state

whichever is clearer.

Answer 16

Although I prefer style 1. I find it useful to use style 2 sometimes as it is helpful to reduce the level of indentation and make the code a bit easier to read.

Either style is good to be honest, it's really down to personal choice.

Answer 17

My preference would be Style 1 but I would use Style 2 in a heartbeat if !parseMsg or !processMsg happened more often than not. Always put the most likely scenario first - right?

Answer 18

If someone later has to maintain this code to add further actions on msg that may or may not depend on whether the message has been parsed and/or processed, it would be simpler to do so if the code was in style 1 than in style 2, so for that reason I'd prefer style 1.

Answer 19

Guard Clause works well for a conditional function return because it creates a complete statement. At one glace you know what is going on(we are done here).

The continue statement generally requires a bit more thought. I personally feel if your using more than one continue statement in a loop you're doing something wrong.

The first type in my opinion is self documenting and the standard way. When ever you go against the standard way you need to add comments.

Answer 20

I find the style 1 more easy to follow. So I prefer style 1.

Answer 21

style 2 lets the human reader focus on the important code, and not even look at anything that isn't relevant - you see the continue statement and anything below is irrelevant.

style 2 manages complexity and thus scales to more complicated methods, style 1 quickly becomes unmanageably complicated.

Answer 22

I want to Refactor this code in a different direction. Your code is doing too many things!

1. Reading input
2. Iterating
3. Raising notifications
4. Parsing (conditionally! (with notification!!))
5. Processing (conditionally! (with notification!!))

I think we need some separation here. I want to explore:

• Moving the reading input in to an iterator (yield return).
• Moving conditions in to the Strategy Pattern

Answer 23

Both choices are awkward. Start with style 2, extract the inside of the while loop into a new method, and change the continue statements to return statements. And it would still need more refactoring.

The continue keyword is awkward, but the return statement is well understood. This option would have almost no nested indentation.