tags:

views:

855

answers:

19

My first attempt of reverse for loop that does something n times was something like:

for ( unsigned int i = n-1; i >= 0; i-- ) {
    ...     
}

This fails because in unsigned arithmetic i is guaranteed to be always greater or equal than zero, hence the loop condition will always be true. Fortunately, gcc compiler warned me about a 'pointless comparison' before I had to wonder why the loop was executing infinitely.


I'm looking for an elegant way of resolving this issue keeping in mind that:

  1. It should be a backwards for loop.
  2. The loop index should be unsigned.
  3. n is unsigned constant.
  4. It should not be based on the 'obscure' ring arithmetics of unsigned integers.

Any ideas? Thanks :)

+8  A: 
for ( unsigned int i = n; i != 0; i-- ) {
    // do something with i - 1
    ...     
}

Note that if you use C++ as well as C, using != is a good habit to get into for when you switch to using iterators, where <= etc. may not be available.

anon
plus, when any code within the loop decreases i even further, you'll notice an infinite loop soon. (and it's easier to reason about)
xtofl
+9  A: 
for ( unsigned int i = n; i > 0; i-- ) {
    ...  
    i-1 //wherever you've been using i   
}
vartec
I personally find that using "i-1" instead of just "i" is non-obvious without a big warning comment and is likely to cause strange bugs when someone in the future instinctively uses "i" instead of "i-1".
Skizz
yeah, just like using 0 to accessing first cell of an array. Life ain't easy ;-)
vartec
You can get around that by calling i 'counter' and declaring a variable called 'index = counter-1'.
xtofl
+8  A: 
for ( unsigned int loopIndex = n; loopIndex > 0; --loopIndex ) {
    unsigned int i = loopIndex - 1;
    ...
}

or

for ( unsigned int loopIndex = 0; loopIndex < n; ++loopIndex ) {
    unsigned int i = n - loopIndex - 1;
    ...
}
Lou Franco
Yikes! I accidentally deleted a comment when trying to delete my own. Got from my cache: "A classic example of long variable names making the code less readable, IMHO." -- sorry, can't find an undo
Lou Franco
+2  A: 
for ( unsigned int i = n; i > 0; i-- ) {
    ...     
}

Should work fine. If you need to use the i variable as an index into an array do it like this:

array[i-1];
arul
+2  A: 
for ( unsigned int i = n; i > 0; i-- ) {
    unsigned int x = i - 1;
    // do whatever you want with x    
}

Certainly not elegant, but it works.

itsmatt
A: 
for ( unsigned int i = n-1; (n-i) >= 0; i-- ) {
    // n-i will be negative when the loop should stop.
    ...     
}
Paulo Guedes
if n and i are both unsigned, n-i will be also unsigned, won't it?
Auron
'mmm, maybe you're right.
Paulo Guedes
+18  A: 

How about:

for (unsigned i = n ; i-- > 0 ; )
{
  // do stuff with i
}

Skizz

Skizz
Pretty good! Derived from http://stackoverflow.com/questions/275994/whats-the-best-way-to-do-a-backwards-loop-in-c-c-c/276056#276056. The only problem I see is that i is never equal to n, which seems counterintuitive looking at the loop declaration.
Auron
@Auron, n is typically the length of an array, so in most cases you don't want i == n.
quinmars
I still feel it requires a _much_ harder brainparse than the solution where you explicitly distinguish between your counter and your index.
xtofl
I support the solution, because once you had brainparsed the "i-- > 0" (which isn't that hard!) you can forget about entire problem and just use i.
As far as I'm concerned this is the standard idiom for reverse loop, I'm surprised people haven't met it before.
bobince
You can also do "for (unsigned i = n ; i-- ; )" but that is starting to look a bit weird, as in, an error - putting the loop update in the wrong place. Putting the "> 0" implies the "i--" is intentional. In my opinion anyway.
Skizz
yeah, this looks neat
Johannes Schaub - litb
It might be nice to mark this kind of construct with a nice comment to help people through the mental speedbump.
Chris Lutz
Wow, awesome! It's constructs like this that make me admire the C/C+++ languages. And also marvel at the ability to shoot yourself in the foot in them. :P
Vilx-
+1  A: 

Hm. Here are your options:

  1. Use i=0 as your break condition - Loop will not execute when i reaches 0, so execute 1 iteration of the loop contents for i=0 after the loop has exited.
for ( unsigned int i = n-1; i > 0; i-- ) {
    doStuff(i);
}
doStuff(0);
  1. In the loop, test for i=0 and break out. Not recommended because now you're testing the value of i twice in the loop. Also using break within a loop is generally regarding as bad practice.
for ( unsigned int i = n-1; i >= 0; i-- ) {
    doStuff(i);
    if (i=0) break;
}
Tim
Your second loop will generate the 'pointless test' warning.
Skizz
I think repeating the doStuff just because you're working with unsigned int is a bit... overhead? Just as testing for one special case of something that's actually not special at all.
xtofl
+4  A: 

I'd tend to use

 for ( unsigned int i = n; i > 0; )  {
    --i;
    ...     
 }

it's almost the same as skizz' answer, (it misses out a final unnecessary decrement, but the compiler should optimise that away), and actually will pass code review. Every coding standard I've had to work with has had a no-mutation in conditional rule.

Pete Kirkham
Now _that_'s a mutilated for construct, to be swiftly replaced by a while statement! -1 for that.
xtofl
xtofl -- No, not a while statement. Skizz shows the best way. A while statement needs its loop variable declared outside its scope, in an independent statement.
Svante
Unfortunately, Skizz' answer wouldn't get past code review in most shops.
Pete Kirkham
"no-mutation in conditional" - I thought we were programmers, not (code)monkeys! Rules should be broken wherever the code would suffer because of it. Using an 'i-1' indexer is far worse than a mutating conditional. The hoops some of these answers jump through are just ugly.
Skizz
I agree about using a i-1 indexer being worse.
Pete Kirkham
+3  A: 

Maybe this way? IMHO its clear and readable. You can omit the if(n>=1) if it is implicitly known somehow.

if(n>=1) {
    // Start the loop at last index
    unsigned int i = n-1;
    do {
       // a plus: you can use i, not i-1 here
    } while( i-- != 0 );
}

Another version:

if(n>=1) {
    unsigned int i = n;
    do {
       i--;

    } while( i != 0 );
}

The first code without if statement would look like:

unsigned int i = n-1;
do {

} while( i-- != 0 );
+1. do while was made for this sort of situation!
Breton
+1  A: 

Since this is not a standard for loop I would probably use a while loop instead, e.g.:

unsigned int i = n - 1;
while (1)
{
    /* do stuff  with i */

     if (i == 0)
    {
        break;
    }
    i--;
}
starblue
A: 

This is untested, but could you do the following:

for (unsigned int i, j = 0; j < n; i = (n - ++j)) {
    /* do stuff with i */
}
SpoonMeiser
+1  A: 
for (unsigned int i = n-1; i<(unsigned int)-1; i--)

OK, its "obscure ring arithmetic".

Eric Bainville
or even -1u instead of (unsigned int)-1
Johannes Schaub - litb
+1  A: 

Or you could rely on the wrapping behaviour of unsigned int if you need indexing from n-1 to 0

for(unsigned int i = n-1; i < n; i--) {
    ...
}
peje
+1  A: 

The only reason I mention this option is because I did not see it in the list.

for ( unsigned int i = n-1; i < n; i-- ) {
... 
}

Totally against intuition, but it works. the reason it works is because subtracting 1 from 0 yields the largest number that can be represented by an unsigned integer.

In general I do not think it is a good idea to work with unsigned integers and arthmetic, especially when subtracting.

Renze de Waal
This makes me wonder if it is a good idea or not to work with unsigned arithmetic in for loops...
Auron
+1  A: 
unsigned index;
for (unsigned i=0; i<n; i++)
{
    index = n-1 - i; // {i == 0..n-1} => {index == n-1..0}
}
A: 

Use two variables, one to count up, and the other for the array index:

unsigned int Index = MAX - 1;
unsigned int Counter;
for(Counter = 0; Counter < MAX; Counter++)
{
    // Use Index
    Index--;
}
Steve Melnikoff
A: 

Easy, just stop at -1:

for( unsigned int i = n; i != -1; --i )
{
 /* do stuff with i */
}

edit: not sure why this is getting downvoted. it works and it's simpler and more obvious than any of the above.

Dave
It doesn't work because `i` is unsigned and you should not be comparing it to a signed value. My C compiler has the warnings turned up high enough not to accept code like this. If you used a cast like `(unsigned)-1` it would work, but why not use the more obvious `UINT_MAX` macro? If I saw this in someone's code as is, I would be scratching my head trying to figure out why it wasn't an infinite loop. It works, but it's not obvious.
Chris Lutz
A: 

e.z:

#define unsigned signed

for ( unsigned int i = n-1; i >= 0; i-- ) { ...
}

Why would I want tho change 'unsigned' by 'signed' and lose all the semantics?
Auron