Elegant way to iterate in C++

Question

Let's say I have a vector of Polygons, where each polygon contains a vector of Points. I have to iterate over all the points of all the polygons many times in my code, I end up having to write the same code over and over again:

for(std::vector<Polygon*>::const_iterator polygon = polygons.begin();
                polygon != polygons.end(); polygon++)
{
        for(std::vector<Point>::const_iterator point = (*polygon)->points.begin();
                        point != (*polygon)->points.end(); point++)
        {
                (*point).DoSomething();
        }
}

I really feel that is a lot of code for two simple iterations, and feel like it's clogging the code and interfering with the readability.

Some options I thought are:

• using #defines - but it would make unportable (to use in other parts of the code). Furthermore, #defines are considered evil nowadays;
• iterate over vector->size() - it doesn't seem the most elegant way;
• calling a method with a function pointer - but in this case, the code that should be inside of the loop would be far from the loop.

So, what would be the most clean and elegant way of doing this?

Answer 1

In C++11, using ranged-base for loops and the auto keyword:

for(const auto& polygon : polygons) {
    for(const auto& point : polygon->points) {
        point.DoSomething();
    }
}

Answer 2

If you can't use C++11, boost has a FOREACH macro which generates a lot of code but dramatically simplifies your code:

BOOST_FOREACH(Polygon * polygon, polygons)
{
    BOOST_FOREACH( Point & point, polygon->points )
    {
        point.doSomething();
    }
}

Answer 3

If you can't use C++11, maybe typedef the iterator type to something shorter like

typedef std::vector<Polygon*>::const_iterator PolyCit;
for (PolyCit polygon = polygons.begin(); polygon != polygons.end(); polygon++)

Answer 4

The inner loop can be rewritten using algorithms like this:

std::for_each(
    (*polygon)->points.begin(), (*polygon)->points.end(), 
    &Point::DoSomething
);

Mixing this with the outer loop is a bit more complex:

std::for_each(
    polygons.begin(), polygons.end(),
    []( Polygon* polygon ) {
        std::for_each(
            polygon->points.begin(), polygon->points.end(), 
            &Point::DoSomething
        );
    }
);

If we had some kind of compound iterator we could really express your intent, which is to do something for each point in each polygon. And a range library like Boost.Range would allow you to avoid naming each container twice, given that you want to work with their entire range.

My ideal version of your code would look like this:

for_each( flatten( polygons ), &Point::DoSomething );

were flatten would return a view of every Point in every Polygon as if it were a single continuous range. Note that this is something that can be accomplished in plain C++03, and all we are missing from Boost.Range to accomplish it is a flatenning range, which shouldn't be difficult to implement in terms of range join.

Otherwise, the range-based for-loop together with auto will help you reduce the boilerplate of iterating throw a range and forgetting about the complicated types.