Hacker News Reader

Comment by augustk | Hacker News Reader

Talking about clear and coherent, does anyone know if there is a deeper reason for using `break' and `continue' instead of pure structured programming? The snippet

  i = ent->client->chase_target - g_edicts;
  do {
     i++;
     if (i > maxclients->value)
        i = 1;
     e = g_edicts + i;
     if (!e->inuse)
        continue;
     if (!e->client->resp.spectator)
        break;
  } while (e != ent->client->chase_target);

for instance, from the function ChaseNext in the file original/rogue/g_chase.c can be reduced to

  i = ent->client->chase_target - g_edicts;
  do {
     i = i % maxclients->value + 1;
     e = g_edicts + i;
  } while ((! e->inuse || e->client->resp.spectator) && (e != ent->client->chase_target));

which in my opinion is clearer since the exit condition for the loop is in one place.

View on HN

I find the latter - with all the conditions much less clear. I have to pause and reason about each condition and it’s relation to the others, the order of the comparisons and the effect of short circuits.

The first example makes it very clear that condition A means do it again and condition B means exit this loop while condition C establishes the boundaries of the loop.

I think it's debateable which is clearer. Your while condition requires some mental parsing of the booleans whereas the original can be analysed one at a time.

I'd say I prefer the original.

One nice property with (pure) structured programming is that the statement

  do {
     ...
  } while (p);

guarantees that p is false after the loop and this makes it easier to reason about the program. With breaks, however, the loop could be "lying" so to speak.

Agreed. I think it's also a little easier to understand what's going on when stepping through the code in a debugger.

For me the original is much clearer and is how I normally structure my iterators/do/for loops, since explicitly checking for conditions and using 'continue' when the iterated value is not worth processing makes it nice and clear that everything _after_ these checks is valid and can contain the meat of the processing without more if/else checks.

This approach also works fine for c++ and c# code (probably also java too), which I like as it keeps my projects nice and consistent and I can jump between codebases and feel at home.

The only thing that I don't like about it is the if() statements not being contained in {}'ies. That always makes me nervous.

Using % vs comparison and reset was probably more efficient with the compilers of the time; might still be.

Also I doubt everyone would agree that the one exit condition is more clear. For example, if I know that if e->inuse is 0 it will continue with the next round. Arguably it's more difficult to understand that from the single combined expression—because it isn't so: if !e->inuse but e == ent->client->chase_target, the original will loop but yours will exit.

Though I'm guessing ent->client->chase_target->inuse is probably never 0.

> if !e->inuse but e == ent->client->chase_target, the original will loop but yours will exit.

No, the continue statement will jump to the test part also for `do' loops:

"The continue statement is related to break, but less often used; it causes the next iteration of the enclosing for, while, or do loop to begin. In the while and do, this means that the test part is executed immediately"

-- "The C programming language (Second Edition)"

Well today I have learned something I may have forgotten, thank you :). It seems natural in for and while so it does make sense also the do loops use it.

Seconded that the main reason was probably avoiding a mod. But it's not because of the compiler, but the processor. Mod and division instructions are always more expensive than additions and multiplications. If this function was on the hot path (and looking at where it's called, it's at least called once per frame), that may have mattered a lot.

A modern optimizing compiler would quite likely do induction variable elimination/strength reduction and replace the % with increment-and-reset if it deemed it more efficient, so it’s also about the compiler.

Pretty sure your line 3 and the original lines 3-5 are not equivalent. Check it with `i` entering the loop with value `maxclients->value`.

The best reason not to re-write for style reasons is you break it.

Thanks, now corrected.

If `i` can enter with value greater than `maxclients->value` (why not), it's still wrong.

Break, continue, return (and arguably even C's goto before variables could be declared in the middle of a scope block) are all entirely normal tools in the structured programming toolbox. How those are used is just personal taste.

Reply on news.ycombinator.com