PDA

View Full Version : [SOLVED] C++ iterator problems. What am I doing wrong here?




chrono1081
Jun 5, 2011, 04:26 PM
Hi guys,

I'm cleaning up some code for a project and I'm running into trouble.

I have a vector of objects called mObjectList which displays primitives to be drawn on screen. I have a function to add new objects to the vector, and am now making a function to remove objects.

I have a crappy way that works but I would really like to do it the proper way by using an iterator, and here is where I am running into problems. I seriously don't know why I am having such a hard time with this (I think I just need to step away from my code but its not an option right now :/ )

Here is where I declare my iterator:

//Iterator to cycle through the mObjectList
vector<GraphicsObject>::const_iterator iter;

And here is my function to remove an element (note the pseudocode section)

void OGLApp::removeElement(GraphicsObject &object)
{
//Pseudocode
for(iter = mObjectList.begin(); iter != mObjectList.end(); ++iter)
{
if(*iter == object)
mObjectList.pop_back();
}
}


There is code I am referencing from a book I have that is doing this exact same thing with no problems, I however receive the error "Invalid operands to binary expression ('GraphicsObject' and 'GraphicsObject').

What I want the code to do is simply compare the object passed to it with its location in the mObjectList vector and delete itself. Anyone have any suggestions?

Also it should be noted I never use vectors so it could be something completely obvious I'm overlooking.

EDIT: I should also note that originally I tried this:

Declare iterator
//Iterator to cycle through the mObjectList
vector<int>::const_iterator iter;

And here is the function to remove the element in the code:


void OGLApp::removeElement(GraphicsObject &object)
{
//Pseudocode
for(iter = mObjectList.begin(); iter != mObjectList.end(); ++iter)
{
if(*iter == object.ID())
mObjectList.pop_back();
}
}


But, this doesn't work either since the objects are not of int type.



gnasher729
Jun 5, 2011, 04:51 PM
void OGLApp::removeElement(GraphicsObject &object)
{
//Pseudocode
for(iter = mObjectList.begin(); iter != mObjectList.end(); ++iter)
{
if(*iter == object.ID())
mObjectList.pop_back();
}
}




pop_back () would always remove the last element of the vector; I doubt that's what you want. And iterators become invalid when you remove an element, so you'll have to check carefully which method you use to remove an object, and what that method returns.

chrono1081
Jun 5, 2011, 04:55 PM
pop_back () would always remove the last element of the vector; I doubt that's what you want. And iterators become invalid when you remove an element, so you'll have to check carefully which method you use to remove an object, and what that method returns.

Thank you so much for your help so far. I found that instead of pop_back I should use erase.

Here is what my code is supposed to do

-Pass an object to the removeElement();

-Cycle through the mObjectList vector and remove the element who's ID matches that of the object passed to the function. (Each object has an ID that is of int type that can be retrieved by using objectname.ID();)

-Continue on as normal.

EDIT:

Here is updated code that does things more correctly, but I'm still getting an error that states:

"Invalid operands to binary expression ('GraphicsObject' and 'GraphicsObject')"

I highlighted in the code where the problem is.

Here is the top part where I declare the iterator:

//Iterator to cycle through the mObjectList
vector<GraphicsObject>::iterator iter;


And here is the bottom section with my iterator

void OGLApp::removeElement(GraphicsObject &object)
{
//Pseudocode
while(iter != mObjectList.end())
{
if(*iter == object) //This is the problem
{
iter = mObjectList.erase(iter);
}
else
{
iter++;
}

}
}


Any ideas how to get rid of this error?

jiminaus
Jun 5, 2011, 05:40 PM
Does GraphicsObject have an operator ==? If so, how is it declared?

chrono1081
Jun 5, 2011, 05:49 PM
Solved!

Thank you all for your help and replies! I really appreciate it :)

I ended up getting it to work by doing the following:


void OGLApp::removeElement(GraphicsObject &object)
{

//Iterator to cycle through the mObjectList
vector<GraphicsObject>::iterator iter;

//Pseudocode
for(iter = mObjectList.begin(); iter != mObjectList.end(); ++iter)
{
if(iter->ID() == object.ID())
{
iter = mObjectList.erase(iter);
iter --;
}
}
}


I really need to learn more about STL. I didn't have to use it for this project but I wanted to because it looks a ton nicer than what I had and I don't like to turn in ugly code if I can help it ;)

Thank you all again :)

jiminaus
Jun 5, 2011, 06:04 PM
I know you marked this thread as solved (should be resolved), but because you said you didn't know STL I though I'd show you the STL way of doing this.

Add an operator == to GraphicsObject like so:

bool GraphicsObject::operator ==(const GraphicsObject& rhs) const
{
return ID() == rhs.ID();
}


Then your removeElement becomes this:

void OGLApp::removeElement(GraphicsObject &object)
{
vector<GraphicsObject>::iterator newEnd =
remove(mObjectList.begin(), mObjectList.end(), object);
mObjectList.erase(newEnd, mObjectList.end());
}


Hmmm, not so elegant. So never mind perhaps. :(

chrono1081
Jun 5, 2011, 07:20 PM
I know you marked this thread as solved (should be resolved), but because you said you didn't know STL I though I'd show you the STL way of doing this.

Add an operator == to GraphicsObject like so:

bool GraphicsObject::operator ==(const GraphicsObject& rhs) const
{
return ID() == rhs.ID();
}


Then your removeElement becomes this:

void OGLApp::removeElement(GraphicsObject &object)
{
vector<GraphicsObject>::iterator newEnd =
remove(mObjectList.begin(), mObjectList.end(), object);
mObjectList.erase(newEnd, mObjectList.end());
}


Hmmm, not so elegant. So never mind perhaps. :(

Thanks so much! I'm still tweaking around in the code so the more tips the merrier! :)

ShortCutMan
Jun 6, 2011, 03:23 AM
I would do it similar to how jiminaus did it, by using std::remove_if with a lambda or nested struct with static function. Implementing operator== may not be something you want available to users of that object depending on what it is used for.

The best way to learn STL is by just seeing if it has a solution for a problem you currently have. If it does, great you learn a new algorithm or container!