Re: Please don't do this (code fragment)

[elf@florence.buici.com]

On Mon, Jan 14, 2002 at 07:01:17AM +0100, Samuel Tardieu wrote:
> On 13/01, elf@florence.buici.com wrote:
> 
> |   int i;
> |   for (i = 0; i > -1; i += 1) {
> |     // ...
> |     if (terminal_condition)
> |       break;	
> |     // ...
> |   }
> [...]
> | Moreover, i is never used.  The loop could be reduced to
> | 
> |   while ((file = fts_read (dir)) != NULL) {
> |     // ...
> |   }
> 
> Those are not equivalent: the first loop, while ugly, has a guard against
> endless looping if fts_read always returns non-NULL for any reason (not
> knowing what fts_read contains, it is hard to tell whether there is a
> reason for this or not).

A poor reason to write that code for several reasons.  

  1) It bounds the loop to the size of the integer /2 without
     explicitly indicating so. 
  2) If there is a problem, it silently hide the fact.
  3) It is non-deterministic.  Should the loop fail to terminate
     properly, the program will run for a relatively long time.  15
     seconds on the PIII 700MHz I'm using.

While I don't agree that this is an appropriate method for
guaranteeing termination, the same result with clearer expression
would be

  int loop_max = MAX_INT;
  while ((file = fts_read (dir)) != NULL && --loop_max) {
  }

I'd rather see the program fail to terminate so that I knew something
was awry instead of imposing an unforseen limitation on the program.
Moreover, the original depends on the fact that (MAX_INT + 1) < 0
which, though true, is a guaranteed result.  Were I to be running a 64
bit CPU, the time to terminate either of these loops would approach
infinity (2^32*15s)->~2042 years.  I don't see the point of using such
a weak attempt to guarantee termination.

Cheers.