Jump to content
  • Advertisement
Sign in to follow this  
nGamer

Style preferences in for loop structure

This topic is 690 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Hey guys,

 

So I think this probably more of a style preference/opinion thread than a question with a specific answer.

 

Let's say you have a for loop that goes through an array, vector, or whatever. Maybe it's a search and destroy or something else, but it has the following form:

for (int i = 0 ; i < MAX ; i ++)
{
    if (someCondition == true)
    {
        foo () ;
        bar () ;
    }
}

Now, afaik, the previous code would be behavior equivalent to:

for (int i = 0 ; i < MAX ; i ++)
{
    if (someCondition == false) {continue ;}
    foo () ;
    bar () ;
}

Is there a predominant form or an advantage in using one over the other?

Share this post


Link to post
Share on other sites
Advertisement
It depends on how many such conditions there are, and what the "most common" case is. If there are several conditions, then I would prefer the second form, because it's easier to follow and to debug. Compare:
 
// first way
for (int i = 0 ; i < MAX ; i ++)
{
    if (someCondition && someOtherCondition && unrelatedCrap && 
        whyIsThisHere && isThereAnyCoffeeLeft)
    {
        foo () ;
        bar () ;
    }
}

// second way
for (int i = 0 ; i < MAX ; i ++)
{
    if (!someCondition)
    {
        continue;
    }
    if (!someOtherCondition )
    {
        continue;
    }
    if (!unrelatedCrap )
    {
        continue;
    }
    if (!whyIsThisHere )
    {
        continue;
    }
    if (!isThereAnyCoffeeLeft)
    {
        continue;
    }

    foo () ;
    bar () ;    
}
The second form is much longer, true, but note that now I can put debug breakpoints on each individual "continue" statements, so when one of the conditions fails I can explicitly see which one it was. Similarly, if I put an uncommon condition in its own if/continue block, then I can put a breakpoint on it and add logging to catch that case. In the "real world", each of those single variables in the condition might be giant expressions in and of themselves, so separating them helps make them more understandable, too.

If there's only one condition and you expect to be the most common one, I would say go with the first one, as it's more straightforward and only one condition that could fail. Edited by Oberon_Command

Share this post


Link to post
Share on other sites

That makes sense. Never thought about it from a debugging standpoint.

Edited by nGamer

Share this post


Link to post
Share on other sites

That's make sense. Never thought about from a debugging standpoint.


No matter how correct your code is, if it goes out into the world, someone will always want to step through it with a debugger. Even if they aren't chasing a bug, some people use the debugger to understand how unfamiliar code works. Don't make it hard on those people - those people might even be you, six months from now. :) Edited by Oberon_Command

Share this post


Link to post
Share on other sites

understand how unfamiliar code works. Don't make it hard on those people - those people might even be you, six months from now.
 

LOL, so very true!

Share this post


Link to post
Share on other sites
for(int i = 0; i < MAX; i ++) {
  if(someExemptionFromWhatFollows) { continue; }
  
  if(someCondition) {
      foo();
      bar();
  }
}

Share this post


Link to post
Share on other sites
Forget the body, what about the fist line? :wink:
for(int i = 0; i < MAX; i ++)
VS idiomatic iterator style:
for(int i = 0, end = MAX; i != end; ++i)

Share this post


Link to post
Share on other sites

Forget the body, what about the fist line? :wink:

for(int i = 0; i < MAX; i ++)
VS idiomatic iterator style:
for(int i = 0, end = MAX; i != end; ++i)

 

The for loop is everything wrong with imperative code imho :) 

It has too many moving parts and too many ways to make stupid mistakes. Much prefer the declarative approach C# allows with something like this

Range(0, MAX).Foreach(i => {do stuff here});

and to filter

 

Range(0, MAX)

    .Where(i => someCondition)

    .Foreach(i => {do stuff here});

Easier to read and reason about and really hard to mess up :)

Share this post


Link to post
Share on other sites

I'm personally a fan of decrementing my for-loops rather than incrementing. So instead of this:

for(int i = 0; i < MAX; i ++) {
    // ...
}

There's this:

for(int i = MAX; i --> 0;) {
    // ...
}

It comes in handy when you're iterating over a container and are deleting elements along the way. This way you don't have to add as much logic for moving to the next element after deleting the one prior.

Share this post


Link to post
Share on other sites
i --> 0

 

I've never seen anyone write it like this, and in fact, without my morning coffee, it took me a few seconds to realize this is actually valid code and I had to put in extra effort to understand that it iterates over the same range. It's a neat trick, but I'm not sure it's really worth confusing other people who may read the code.

Edited by rnlf

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!