Re: Please don't do this (code fragment)
Craig Dickson <crdic@yahoo.com> writes:
> Richard Kettlewell wrote:
>> Even if you don't care about weird platforms, "x > -1" is a
>> ridiculously obscure test in this context; to achieve the same
>> effect it would be much clearer to make x unsigned and do "x <=
>> (unsigned)INT_MAX".
>
> I find "x <= (unsigned) INT_MAX" to be more obscure than the
> original. When I first glanced at it, I thought, "That's dumb, x is
> ALWAYS less than or equal to INT_MAX by definition!"
> Then my second thought was that the cast on the constant will cause
> x to also be converted to unsigned. In contrast, when I saw "x >
> -1", I immediately realized it was testing for wraparound.
By 'make x unsigned' I mean 'declare as "unsigned int x;"'.
> To achieve almost the same effect (probably close enough in this
> situation), it would be better to simply test for "x < INT_MAX" -- it's
> clearer than either the original or your cast-dependent version, and
> only stops one iteration sooner. Since in this case the actual number of
> iterations was not the point (rather, merely that the loop should be
> guaranteed to terminate eventually), it ought to be sufficient.
If we're allowed to change the meaning, there are much more sensible
options still. That wasn't really the point of my post.
--
http://www.greenend.org.uk/rjk/
Reply to: