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

You can throw braces around the initialization and loop if you are concerned about scope. Often what I'll do is declare iterators at the start of the function and reuse them throughout the program.

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.

Answer 3

I agree with Ferruccio. The first style might be preferred by some in order to pull the end() call out of the loop.

I might also add that C++0x will actually make both versions much cleaner:

for (auto iter = container.begin(); iter != container.end(); ++iter)
{
   ...
}

auto iter = container.begin();
auto endIter = container.end();
for (; iter != endIter; ++iter)
{
   ...
}

Answer 4

I would usually write:

SomeClass::SomeContainer::iterator Iter = m_SomeMemberContainerVar.begin(),
                                   IterEnd = m_SomeMemberContainerVar.end();

for(...)

Answer 5

No, it's a bad idea to get a hold on iter.end() before the loop starts. If your loop changes the container then the end iterator may be invalidated. Also, the end() method is guaranteed to be O(1).

Premature optimization is the root of all evil.

Also, the compiler may be smarter than you think.

Answer 6

I don't have a particularly strong opinion one way or the other, though iterator lifetime would lean me toward the for-scoped version.

However, readability may be an issue; that can be helped by using a typedef so the iterator type is a bit more manageable:

typedef SomeClass::SomeContainer::iterator sc_iter_t;

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

Not a huge improvement, but a bit.