elf@florence.buici.com wrote:
> 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.
Samuel wasn't defending the original code; he was just observing that
your suggested replacement was not functionally equivalent. Which it
wasn't.
> 1) It bounds the loop to the size of the integer /2 without
> explicitly indicating so.
It's pretty clear if you understand how twos-complement arithmetic
works. Which is not to say it's the right thing to do, but I've seen
code that is much harder than this to understand.
> 2) If there is a problem, it silently hide the fact.
It's impossible to know that based on the code fragment you posted.
There's no indication of what happens after the loop, or even most of
the inside of the loop. I'm sure you're familiar with the rest of the
code, but the rest of us probably aren't.
> 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.
The original code is deterministic (the loop will definitely exit sooner
or later); your suggested replacement is not, unless it is guaranteed
that fts_read will eventually return NULL.
> 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) {
> }
That's clearer, yes. It still sucks, unless there's a really good reason
to use MAX_INT as the limit. It's hard to say without knowing more about
the context of this tiny fragment of code, but I would guess that since
the loop condition is only used to guarantee eventual termination, the
limit could reasonably be set to a constant that is consistent across
platforms (whatever that constant might be -- 10^3, 10^6, 10^9,
whatever), rather than one that will vary by multiple orders of
magnitude depending on whether the target system uses 16-, 32-, or
64-bit ints.
> 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.
I'd rather see the program send a useful diagnostic message to stderr or
a log file, and either exit or properly recover from the error.
Craig
Attachment:
pgph1qGCZDhsF.pgp
Description: PGP signature