CODEWITHSUNDEEP
c++vectorcoding-style
André Wagner
André Wagner

Reputation: 1477

Elegant way to iterate in C++

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:

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

Upvotes: 7

Views: 1589

Answers (7)

bobobobo
bobobobo

Reputation: 67224

First of all, there's the plain old integer-based loop. A bit shorter than iterators.

for( int i = 0 ; i < polygons.size() ; i++ )
{
    for( int j = 0 ; j < polygons[i]->points.size(); j++)
    {
        Point* p = polygons[i]->points[j] ;
        p->DoSomething();
    }
}

If you don't like that, and you don't have C++11 available, you can write function that accepts a functor (these were in C++0x under std::tr1 I believe):

void eachPoint( function<void (Point* p)> func )
{
    for( int i = 0 ; i < polygons.size() ; i++ )
    {
        for( int j = 0 ; j < polygons[i]->points.size(); j++)
        {
            Point* p = polygons[i]->points[j] ;
            func(p);
        }
    }
}

Alternatively, a plain old macro: 

#define EACH_POLYGON( polyCollection ) for( int _polyI = 0 ; _polyI < polyCollection.size() ; _polyI++ ) \
for( int _ptNo = 0, Point* p=polyCollection[_polyI]->points[0] ; j < polyCollection[_polyI]->points.size() && (p=polyCollection[_polyI]->points[_ptNo]); _ptNo++)

EACH_POLYGON( polyCollection )
{
    p->DoSomething();
}

Upvotes: 0

pelletjl
pelletjl

Reputation: 505

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();
    }
}

Upvotes: 9

Benjamin Lindley
Benjamin Lindley

Reputation: 103693

Nevermind, this won't work for you since you have a vector of pointers at the top level, but I'll keep it up, because I think it's pretty cool.

My template metaprogramming is a bit rusty, so there might be a simpler way to do this, but:

template<typename C, typename F, size_t depth>
struct nested_for_each_helper
{
    static void do_it(C& c, F& f)
    {
        for (auto& i : c)
            nested_for_each_helper<decltype(i),F,depth-1>::do_it(i,f);
    }
};

template<typename C, typename F>
struct nested_for_each_helper<C,F,0>
{
    static void do_it(C& c, F& f)
    {
        f(c);
    }
};

template<size_t depth, typename C, typename F>
void nested_for_each(C& c, F& f)
{
    nested_for_each_helper<C,F,depth>::do_it(c,f);
}

int main()
{        
    int n[3][3][3][3];
    int i = 0;
    nested_for_each<4>(n,[&i](int& n) { n = i++; });
    nested_for_each<4>(n,[](int n){
        std::cout << n << ' ';
    });
}

For your case, you can use it like this (no you can't):

nested_for_each<2>(polygons, [](Point const& p) { p.DoSomething(); });

Upvotes: 1

K-ballo
K-ballo

Reputation: 81349

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.

Upvotes: 5

Pete Becker
Pete Becker

Reputation: 76245

You need a layer of abstraction. Instead of dealing with a vector of polygons, write a class that manages that vector. The class then provides iterator pairs for iterating over the points. The code in the iterator knows and encapsulates those details.

Upvotes: 1

src
src

Reputation: 551

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

Upvotes: 5

Robᵩ
Robᵩ

Reputation: 168626

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();
    }
}

Upvotes: 24

Related Questions