views:

675

answers:

3

Hey there! I'm doing this project and right now I'm trying to:

  1. create some of objects and store them in vectors, which get stored in another vector V
  2. iterate through the vectors inside V
  3. iterate through the objects inside the individual vectors

Anyway, I was just searching the web and I came accross the stl for_each function. It seems pretty neat but I'm having problems with it. I'm trying to use it in this way:

for_each(V.begin(), V.end(), iterateThroughSmallVectors);

the iterateThroug.... simply does the same on the vector passed to it..

Now I'm getting a weird "Vector iterators incompatible" runtime error. I've looked on it and can't find any useful input on this..

I don't know if it helps, but V is a private vector<> stored in class A, which has an accessor to it, and I'm trying to iterate through it in class B by doing:

A->getV().begin(), A->getV().end(), etc..

Anyone got any idea of what is going on?

EDIT: Ok, so I think it is better to just post the code, and where problems might be arrising...

getTiles in gameState.h:

vector<vector<tile*>> getTiles();

for_each loops in main.cpp:

for_each(currState->getTiles().begin(),currState->getTiles().end(), drawTiles);
.
.
void drawTiles(vector<tile*> row)
{
for_each(row.begin(), row.end(), dTile);
}
void dTile(tile *t)
{
t->draw();
}

creating the vectors:

int tp = -1;
int bCounter = 0;
int wCounter = 0;
for (int i = 0; i < 8; i++)
{
vector<tile*> row(8);
    for (int j = 0; j < 8; j++)
    {
 tile *t = new tile(tp, (i+(SIDELENGTH/2))*SIDELENGTH,
     (j+(SIDELENGTH/2))*SIDELENGTH);
 row.push_back(t);
            tp *= -1;
    }
currState->setTiles(row);
    tp *= -1;
}

and just in case it might be relevant:

void gameState::setTiles(vector<tile*> val)
{
    tiles.push_back(val);
}

Is it easier to spot the problem now? I hope so... And if you do spot any stupid stuff I might be doing, please let me know, I'm kind of new to C++ and the pointers and references still confuse me.

EDIT2: Thanks guys, that worked perfectly... well for that problem, now it seems I have an issue with the creation of the tiles and stroing them in the row vector.. it seems that even through the vector is created and passes correctly, the tiles that were supposed to be in it aren't (they are lost after the :

    for (int j = 0; j < 8; j++)
    {
 tile *t = new tile(tp, (i+(SIDELENGTH/2))*SIDELENGTH,
     (j+(SIDELENGTH/2))*SIDELENGTH);
 row.push_back(t);
            tp *= -1;
    }

loop. If any of you has any good ideas about solving this you're welcome to help me ;) In the mean time, I'll keep trying to fix it

A: 

What i do is this: the direct way

vector<vector<int> > vvi;
vector<vector<int> >::iterator vvi_iterator;
vector<int>::iterator vi_iterator;

for(vvi_terator = vvi.begin();vvi_iterator!=vvi.end();++vvi_iterator) {
    for(vi_iterator = (*vvi_iterator).begin();vi_iterator!=(*vvi_iterator).end();++vi _iterator) {
     cout<<*vi_iterator<<" ";
    }  
}

This is the rough idea. I find the for_each method cumbersome for just doing a double-loop. for_each is useful when you want to really do some computation on each element (like some kind of mapping for each element)

ajay
+7  A: 

What's the prototype for A::getV()?

I'm only speculating but if A::getV() doesn't return a reference then it can explain the "Vector iterators are incompatible" error message.

Indeed A->getV().begin() and A->getV().end() would be two iterators over different vectors: each A->getV() invocation returning a different copy of the private member.

Hope this will help you debug your problem.


EDIT: it looks like I anticipated it right: after editing your question providing details, I can see you're defining

vector<vector<tile*> > getTiles();

As a consequence, in the following statement:

for_each(currState->getTiles().begin(),currState->getTiles().end(), drawTiles);

As anticipated above, each call to getTiles() will return a separate temporary copy of the member vector. As a consequence the iterators returned from begin() and end() come from different vectors, hence the error message you're facing at runtime.

Also, as pointed by Charles in his detailed answer, these temporary vectors will be destroyed by the time the function body of for_each is reached.

Consider returning the vector by const reference like this:

const vector<vector<tile*> >& getTiles() const;

And you may as well change drawTiles to avoid even more copies:

void drawTiles(const vector<tile*>& row)

Gregory Pakosz
+1  A: 

You have a couple of serious errors, but first a minor one.

vector<vector<tile*>> getTiles();

Until the next standard comes out you need a space between the to >.

vector< vector<tile*> > getTiles();

This function returns a vector by value which means that it creates a new copy of whatever vector is passed to the return statement in the function. (I assume that this function declaration is the whatever class curState is an instance of.)

When you then do:

for_each(currState->getTiles().begin(),currState->getTiles().end(), drawTiles);

Each call to getTiles will return a separate temporary copy of a vector. Not only does this mean that your iterators from begin() and end() come from difference vectors, but the vectors will be destroyed by the time the function body of for_each is reached.

It looks like you need to research references and pass by reference, because you need to understand these before you can correctly use std::for_each in these scenarios.

Charles Bailey
which basically is what I explained ;)
Gregory Pakosz
@Gregory Pakosz: At the time I started this answer, you hadn't seen the edit with the 'real' code. Perhaps you want to steal the warning about temporaries (I think it's an important point to understand) and the `>>`/`> >` for your answer?
Charles Bailey
@Charles - Sure, from my part it was just a "great minds think alike" amused feeling. I edited the answer to incorporate details from yours about temporaries being destroyed at the time the body of `for_each` is reached; as well as a link to it so that people read your comment about `>>/> >`. You definitely have my vote!
Gregory Pakosz