Sign in to follow this  
nGamer

Style preferences in for loop structure

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
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

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
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.

http://stackoverflow.com/questions/1642028/what-is-the-name-of-the-operator-in-c/27672749#27672749

Share this post


Link to post
Share on other sites

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.

 

 

Well you can still add a conditional breakpoint and then in the 2nd case it becomes.

for(...){
     if(a || b || c || d || e || f || g || z) continue;
}

Share this post


Link to post
Share on other sites

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?

 

The code would probably be optimized anyway (Depends on your build), so what's left to discuss is code readability. 

 

I like the second way because it reduces spaghetti code (which is always easier to read).

The thing I don't like is the usage of the NOT operand, The condition states "Not this, nor/nand that, nor/nand that, continue looping".

Which in terms of logical structure is a bit more difficult to understand right away.

The end up result should be a quick and easy reading, therefore taking a step forward to seperate each condition into a seperate function,

So the result:

- No spaghetti code.

- Easier to read.

- Easier to debug (Breakpoints inside functions).

- Therefore easier to maintain and upgrade.

Share this post


Link to post
Share on other sites

 

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?

 

The code would probably be optimized anyway (Depends on your build), so what's left to discuss is code readability. 

 

I like the second way because it reduces spaghetti code (which is always easier to read).

The thing I don't like is the usage of the NOT operand, The condition states "Not this, nor/nand that, nor/nand that, continue looping".

Which in terms of logical structure is a bit more difficult to understand right away.

The end up result should be a quick and easy reading, therefore taking a step forward to seperate each condition into a seperate function,

So the result:

- No spaghetti code.

- Easier to read.

- Easier to debug (Breakpoints inside functions).

- Therefore easier to maintain and upgrade.

 

I can see where you're coming from with the inverted logic. Being experienced in EE and CE fields, I deal with logic gates a lot. Just like in software, you have AND-OR logic, and NAND-NOR logic. We tend to use inverted logic because it is functionally complete, but, you'd have to flip everything upside down, which doesn't really help understandability. I frequently come to a junction in my code where I wonder if I should sacrifice compactness for readability. For example, the all-time classic:

if (condition == true)
...
if (condition == false)

vs.

if (condition)
...
if (! condition)

Back in my unexperienced days, I would use the latter because all the cool kids were doing it :cool:, but I'm find myself using the first form a lot more to make it easier to read. The more I code, it seems the more I work towards readability.

Edited by nGamer

Share this post


Link to post
Share on other sites

often times you iterate over a list, processing just some elements that meet certain criteria.

 

in such cases the use of continue or return  or break to trap out / skip list entries that do not meet the criteria results in less nesting of code. often it results in a number of checks with continues, followed by the code that processes matching entries at the end of the loop.  i also prefer the simple form of the for loop for readability.

 

 

for i=0 i<max i++

    {

    if (condition A) continue

    ....

    if (condition n) continue

    // its a match! process it!

    do_stuff_with(i)

    }

Edited by Norman Barrows

Share this post


Link to post
Share on other sites

I can see where you're coming from with the inverted logic. Being experienced in EE and CE fields, I deal with logic gates a lot. Just like in software, you have AND-OR logic, and NAND-NOR logic. We tend to use inverted logic because it is functionally complete, but, you'd have to flip everything upside down, which doesn't really help understandability. I frequently come to a junction in my code where I wonder if I should sacrifice compactness for readability. For example, the all-time classic:

 

if (condition == true)
...
if (condition == false)

vs.

if (condition)
...
if (! condition)

Back in my unexperienced days, I would use the latter because all the cool kids were doing it :cool:, but I'm find myself using the first form a lot more to make it easier to read. The more I code, it seems the more I work towards readability.

 

 

Well for me it is not a question of compactness vs readability, I'm standing wtih a simple idioligy , "be as simple as you can".

If compactness doesn't hurt simplicity, make it compact, if readability makes it more simple (And probably will), make it less compact. 

In the end as a developer that uses high languages, I don't care if I put another byte in the code segment. 

Share this post


Link to post
Share on other sites
Speedwise, get rid of the conditionals in any way possible (even - or especially - if this means restructuring your data).

Branches in for loops are bad juju. They typically play very very poorly with branch predictors. Branch prediction is a _massively_ important part of how CPUs work; they're up there with cache access pattern in terms of "hardware crap you really need to understand and think about" when writing games (or any application, imo - even if raw speed isn't important to you, saving on energy costs is hugely important in mobile and server workloads).

With a lot of game loops, for example, you can often achieve this in various ways. For example, here's a _huge_ anti-pattern that you should stamp out of your codebase (for non-trivial projects - it's perfectly fine for small games that only have a few dozen game objects at a time):

for each entity:
  if entity is bullet:
    do bullet stuff
  else if entity is npc:
    do ai stuff
That's bad. Bad bad bad. You want:

for each bullet:
  do bullet stuff
for each bpc:
  do ai stuff
A virtual update method is superior to the if-checks, despite some misguided advice I see thrown around from the peanut gallery on /r/gamedev and the like (though virtual update methods are still way less than ideal, certainly).

A good component-based design does this for you by moving the update logic into the components (or "systems" if you go the ECS route) and then you can group the components into collections together and enumerate only the relevant objects.

This applies to any loop that is executed frequently in your game: logic updates, AI updates, physics updates, particle updates, etc. Reorganize your data so that you're using loops that are inner branch-free (or at least only using predictable branches).

To get to the OP's question: I prefer neither, because you shouldn't be using either the `continue` or `break` (or `if`) keywords in your loops often enough for this question to be worth asking. And when you do need a loop, consider Sean Parent's advice and avoid using "raw" loops.

Share this post


Link to post
Share on other sites

Speedwise, get rid of the conditionals in any way possible (even - or especially - if this means restructuring your data).

Branches in for loops are bad juju. They typically play very very poorly with branch predictors. Branch prediction is a _massively_ important part of how CPUs work; they're up there with cache access pattern in terms of "hardware crap you really need to understand and think about" when writing games (or any application, imo - even if raw speed isn't important to you, saving on energy costs is hugely important in mobile and server workloads)....

I completely understand and respect your argument against branches in loops. I mean I've designed and built computer systems on the circuit level and understand machine cycles, energy consumption, registers and such, but don't you think that "branches in for loops are bad juju" is a bit excessive for the general case? If you're iterating over 300,000 objects, then I can see where you can shave off a substantial amount of time eliminating branches. However, with today's PC hardware standard, would looping through 10 or 15 items with branches make a difference you should care about?

Share this post


Link to post
Share on other sites

Speedwise, get rid of the conditionals in any way possible (even - or especially - if this means restructuring your data).

Branches in for loops are bad juju. They typically play very very poorly with branch predictors. Branch prediction is a _massively_ important part of how CPUs work; they're up there with cache access pattern in terms of "hardware crap you really need to understand and think about" when writing games (or any application, imo - even if raw speed isn't important to you, saving on energy costs is hugely important in mobile and server workloads)....

I completely understand and respect your argument against branches in loops. I mean I've designed and built computer systems on the circuit level and understand machine cycles, energy consumption, registers and such, but don't you think that "branches in for loops are bad juju" is a bit excessive for the general case? If you're iterating over 300,000 objects, then I can see where you can shave off a substantial amount of time eliminating branches. However, with today's PC hardware standard, would looping through 10 or 15 items with branches make a difference you should care about?


It isn't just the performance considerations. Pre-sorting objects to iterate through them without conditions simplifies the code, as well. You can look at a piece of code and know exactly where it's going to go just by looking at it. That (usually) makes it easier to see what a given piece of code is supposed to do in addition to what it does.
 
// this is annoying to read and debug - I have to step through the loop to see what kind of entity I'm on at any given type
for (auto& e : entities)
{
   if (e.type == boop) { // ... 
   }
   if (e.type == snoot){ // ... 
   }
}

// this is better
for (auto& e : boops)
{
  // ...
}
for (auto& e : snoots)
{
  // ...
}
Edited by Oberon_Command

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 :)

 

The only problem with the C# Linq style is that it is significantly slower if you get into a performance critical tight loop. (https://jackmott.github.io/programming/2016/08/20/when-bigo-foolsya.html)  I'm not sure how much of that is inherent versus implementation-specific, but it is something you may need to think about.

Share this post


Link to post
Share on other sites

[

if (condition == true)
...
if (condition == false)
vs.
if (condition)
...
if (! condition)
Back in my unexperienced days, I would use the latter because all the cool kids were doing it :cool:, but I'm find myself using the first form a lot more to make it easier to read. The more I code, it seems the more I work towards readability.

 

That suggests that you're naming your variables poorly. A properly written if() should read similarly to English:

if(dude.isAbiding()) { ... } //"if dude is abiding" ...

if(!moving) { ... } //"if not moving" ...

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this