Is Iterator initialization inside for loop considered bad style, and why?

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

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 2

Another alternative is to use a foreach macro, for example boost foreach:

BOOST_FOREACH( ContainedType item, m_SomeMemberContainerVar )
{
   mangle( item );
}

I know macros are discouraged in modern c++, but until the auto keyword is widely available this is the best way I've found to get something that is concise and readable, and still completely typesafe and fast. You can implement your macro using whichever initialization style gets you better performance.

There's also a note on the linked page about redefining BOOST_FOREACH as foreach to avoid the annoying all caps.