Is Iterator initialization inside for loop conside

Question

Typically you will find STL code like this:

for (SomeClass::SomeContainer::iterator Iter = m_SomeMemberContainerVar.begin(); Iter != m_SomeMemberContainerVar.end(); ++Iter)
{
}

But we actually have the recommendation to write it like this:

SomeClass::SomeContainer::iterator Iter = m_SomeMemberContainerVar.begin();
SomeClass::SomeContainer::iterator IterEnd = m_SomeMemberContainerVar.end();
for (; Iter != IterEnd; ++Iter)
{
}

If you're worried about scoping, add enclosing braces:

{
    SomeClass::SomeContainer::iterator Iter = m_SomeMemberContainerVar.begin();
    SomeClass::SomeContainer::iterator IterEnd = m_SomeMemberContainerVar.end();
    for (; Iter != IterEnd; ++Iter)
    {
    }
}

This is supposed to give a speed and efficiency gain, especially if you are programming consoles, because the .end() function is not called on each iteration of the loop. I just take the performance improvement for granted, it sounds reasonable but i don't know how much and it certainly depends on the type of container and actual STL implementation in use. But having used this style for a couple months now i actually prefer it over the first anyway.

The reason being readability: the for line is neat and tidy. With qualifiers and member variables in real production code it is quite easy to have really long for lines if you use the style in the first example. That's why i intentionally made it to have a horizontal scrollbar in this example, just so you see what i'm talking about. ;)

On the other hand, you suddenly introduce the Iter variables to the outer scope of the for loop. But then, at least in the environment i work in, the Iter would have been accessible in the outer scope even in the first example.

What is your take on this? Are there any pro's to the first style other than possibly limiting the scope of Iter?

Answer 1

The first form (inside the for loop) is better if the iterator is not needed after the for loop. It limits its scope to the for loop.

I seriously doubt that there is any efficiency gain either way. It can also be made more readable with a typedef.

typedef SomeClass::SomeContainer::iterator MyIter;

for (MyIter Iter = m_SomeMemberContainerVar.begin(); Iter != m_SomeMemberContainerVar.end(); ++Iter)
{
}

I would recommend shorter names ;-)

Answer 2

I don't think it's bad style at all. Just use typedefs to avoid the STL verbosity and long lines.

typedef set<Apple> AppleSet;
typedef AppleSet::iterator  AppleIter;
AppleSet  apples;

for (AppleIter it = apples.begin (); it != apples.end (); ++it)
{
   ...
}

Spartan Programming is one way to mitigate your style concerns.

Answer 3

Having looked at this in g++ at -O2 optimisation (just to be specific)

There is no difference in the generated code for std::vector, std::list and std::map (and friends). There is a tiny overhead with std::deque.

So in general, from a performance viewpoint it makes little difference.

Answer 4

If you wrap your code into lines properly, the inline form would be equally readable. Besides, you should always do the iterEnd = container.end() as an optimization:

for (SomeClass::SomeContainer::iterator Iter = m_SomeMemberContainerVar.begin(),
    IterEnd = m_SomeMemberContainerVar.end();
    Iter != IterEnd;
    ++Iter)
{
}

Update: fixed the code per paercebal's advice.

Answer 5

I don't have any console experience, but in most modern C++ compiliers either option ends up being equivilent except for the question of scope. The visual studio compilier will virtually always even in debug code put the condition comparison in an implicit temporary variable (usually a register). So while logically it looks like the end() call is being made through each iteration, the optimized compiled code actually only makes the call once and the comparison is the only thing that is done each subsiquent time through the loop.

This may not be the case on consoles, but you could unassemble the loop to check to see if the optimization is taking place. If it is, then you can you whatever style you prefer or is standard in your organization.

Answer 6

It may make for disjointed code, but I also like to pull it out to a separate function, and pass both iterators to it.

doStuff(coll.begin(), coll.end())

and have..

template<typename InIt>
void doStuff(InIt first, InIt last)
{
   for (InIt curr = first; curr!= last; ++curr)
   {
       // Do stuff
   }
 }

Things to like:

• Never have to mention the ugly iterator type (or think about whether it's const or not-const)
• If there is gain from not calling end() on each iteration, I'm getting it

Things to not like:

• Breaks up the code
• Overhead of additional function call.

But one day, we'll have lambdas!