CODEWITHSUNDEEP
c++ccoding-style
Ori Popowski
Ori Popowski

Reputation: 10672

Style question about existing piece of code (C/C++)

I just hope the following doesn't seem to you like redundant jabber :)
Anyway, there is that:

for (p = fmt; *p; p++) {
    if (*p != '%') {
        putchar(*p);
        continue;
    }
    switch (*++p) {
        /* Some cases here */
        ...
    }
 }

And I wondered why the writer (Kernighan / Ritchie) used the continue in the if statement.
I thought it was for the mere reason that he deemed it would be more elegant than indenting the whole switch under an else statement, what do you think?

Upvotes: 7

Views: 488

Answers (11)

Ori Pessach
Ori Pessach

Reputation: 6843

Probably. The human brain has limited stack space, making it difficult to deal with deeply nested structures. Anything that flattens the information we're expected to parse makes it easier to understand.

Similarly, I normally prefer this:

bool foo(int arg)
{
    if(!arg) {
        /* arg can't be 0 */
        return false; 
    }

    /* Do some work */
    return true;
 }

To this:

 bool foo(int arg) 
 { 
     if(!arg) {
         /* arg can't be 0 */ 
         return false; 
     } else {
         /* Do some work */ 
         return true;
     } 
 }

Or worse, to this:

bool foo(int arg) 
{ 
    if(arg) {
        /* Do some work */ 
        return true;
    } else {
        /* arg can't be 0 */ 
        return false; 
    } 
}

In the last example, the part that does the work might be quite long. By the time the reader gets to the else clause, he may not remember how he got there.

Putting the bail out conditions as close to the beginning helps to assure that people who try to call your functions will have a good idea of what inputs the function expects.

Also, as others pointed out, the continue makes it clear that there's no need to read further into the code inside the loop to determine whether any more processing is done after that point for this case, making the code easier to follow. Again, the fewer things you force the reader to keep track of, the better.

Upvotes: 15

fortran
fortran

Reputation: 76157

I stick to Dijkstra's teachings: goto is harmful. And continue/break are goto's little brothers.

If the problem is that you're indenting the code too much, the solution is not putting a continue in the loop, but reducing the complexity by separating the code in different functions or thinking about a better way of organizing it.

For example, @Kamarey snippet would be even clearer like this:

loop
{
   if (!(cond1 ||
         cond2 ||
         cond2 || 
         ...))
   {
      the loop actions;
   }
}

or @Ori Pessach example could be expressed like:

bool foo(int arg)
{
    if(arg) {
        /*do some work*/
    }

    return arg != 0;
}

In brief, usually I can't find a good enough reason to use non structured programming constructs (maybe for some cleanup codes in a very limited ocasions...)

Upvotes: 0

Sylvain Rodrigue
Sylvain Rodrigue

Reputation: 4994

Well, I wrote C programs for about 11 years and I had to read 5 times your piece of code to understand it !

Kernighan and Ritchie were active in the sixties. At that time, being able to understand a piece of code was not relevant. Being able to write code that fit in 16 Ko was.

So I'm not suprised. C is a terrible language when your teatchers are K & R. Just look at realloc : who would know code something like that today ? In the '60ies, it was all the rage, but it is now appalling, at least :o)

Upvotes: -2

Kamarey
Kamarey

Reputation: 11079

There could be more that one reason to continue/break a loop. So it would look next:

loop
{
   if (cond1)
   {
      if (cond2)
      {
         if (cond2)
         {
            more conditions...
         }
      }
   }
   else
   {
      the loop action 
   }
}

IMHO it's not so elegant and readable as the loop in your example, e.g:

loop
{
   if (cond1)
      continue;
   if (cond2)
      continue;
   if (cond2)
      continue;   
   if( more conditions...)
      continue;

  the loop action 
}

And you don't even need to understand all structure of all "if"s (it could be much more complex) to understand the loop logic.

P.S. just for the case: I don't think the authors thought about how to write this loop, they just wrote it:)

Upvotes: 1

Fionn
Fionn

Reputation: 11305

Because with the continue it is clear that the code is done for this loop iteration. If a else would have been used you had also to check if there is no code after the else.

I think it is general a good habit to exit a context as soon as possible because this leads to much clearer code.

For example:

if(arg1 == NULL)
  return;

if(arg2 == NULL)
  return;

//Do some stuff

vs.

if(arg1 != null)
{
  if(arg2 != null)
  {
    //Do some stuff
  }
}

Upvotes: 9

JimG
JimG

Reputation: 1782

I think that he would have reasons enough to indent the code under the switch, and indenting the entire meat of the function is quite wasteful of horizontal space. At the time the code was written, I imagine 80 character widths were still popular.

I don't think it is difficult to understand, but I do think that it's quite nice to mention what you DON'T do immediately, and then GTFO.

Upvotes: 2

Nikolai Fetissov
Nikolai Fetissov

Reputation: 84239

The most probable reason is that the switch that follows is rather long - this looks like printf format parsing.

Upvotes: 1

ChrisW
ChrisW

Reputation: 56133

If you use an else then everything inside the else needs to be indented:

if ()
{
  doA();
}
else
{
  doB();
  if ()
  {
    doC();
  }
  else
  {
    doD()
  }
}

If you use continue then you don't need to indent:

if ()
{
  doA()
  continue;
}
doB();
if ()
{
  doC();
  continue;
}
doD();

Also, continue means that I can stop thinking about that case: for example, if I see else then perhaps there'll be more processing of the '%' case later in the loop, i.e. at the end of the else statement; whereas on seeing continue I know instantly that the processing of the '%' case in the loop is completely finished.

Upvotes: 1

Peter Perháč
Peter Perháč

Reputation: 20792

It is just so much easier to read when it's put like this.

Are we done here with this iteration through the loop? Yes? So let us continue with the next iteration.

Upvotes: 2

Alex Shnayder
Alex Shnayder

Reputation: 1362

I agree. But you can't look at it as a "mere reason", it's actually a pretty good reason, because it reduces the over all complexity of the code. Making it shorter and easier to read and understand.

Upvotes: 1

Reed Copsey
Reed Copsey

Reputation: 564931

There are always many ways to write code like this -

Putting the entire switch inside an else statement would be perfectly valid. I suppose the reason they did it this way ~may~ have been just the way they were thinking at the time:

"if the value at p does not equal '%', put then continue on."

If you have switch under an else, it may not have been as obvious to the writer that you were jumping to the next iteration in that specific case.

This is completely personal style choices, though. I wouldn't worry too much - just write it in a way that makes the most sense to you and your team.

Upvotes: 1

Related Questions