Sign in to follow this  
Chrono1081

[SOLVED] Iterators in C++. What am I doing wrong?

Recommended Posts

Chrono1081    108
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:

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

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

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

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
[CODE]//Iterator to cycle through the mObjectList
vector<int>::const_iterator iter;[/CODE]

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

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

But, this doesn't work either since the objects are not of int type and I get a warning saying = isn't overloaded which kind of suggests to me that this iterator isn't doing what I think its doing...(or more precisely, the mObjectList.being() isn't returning an int value).

Share this post


Link to post
Share on other sites
Pitterbai    103
[quote name='Chrono1081' timestamp='1307310303' post='4819872']
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:

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

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

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

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
[CODE]//Iterator to cycle through the mObjectList
vector<int>::const_iterator iter;[/CODE]

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

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

But, this doesn't work either since the objects are not of int type and I get a warning saying = isn't overloaded which kind of suggests to me that this iterator isn't doing what I think its doing...
[/quote]

Just at a glance, vector.pop_back() removes the last object of the vector, not the one that your iterator is currently at. You have to delete the member that the iterator is pointing to:

[CODE]
for(iter = mObjectList.begin(); iter != mObjectList.end(); ++iter)
{
if(*iter == object)
{
mObjectList.erase(iter); //Deletes the object that the iterator is pointing to
iter--; //Keep the iterator in the same position so that no objects get skipped. Or, in your case, you can just break here.
}
}
[/CODE]

Share this post


Link to post
Share on other sites
Nanoha    2682
use erase, not pop_back. Also use a hwile loop and not a for loop when erasing. Off the top of my head:

[code]vector::iterator<int> it = someInts.begin();

while(it != someInts.end())
{
if(*it == someInt)
{
// Found the one we want
it = someInts.erase(it);
}
else
{
it++;
}
}[/code]

erase returns an iterator to the next element (hence why we don't iterate if we've erased). I guess you could just break out after erasing.

Share this post


Link to post
Share on other sites
Chrono1081    108
Thank you both for the help and tips so far :) Its greatly appreciated.


Here is updated code that does things more correctly, but unfortunately 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:
[CODE]
//Iterator to cycle through the mObjectList
vector<GraphicsObject>::iterator iter;
[/CODE]

And here is the bottom section with my iterator
[CODE]
void OGLApp::removeElement(GraphicsObject &object)
{
//Pseudocode
while(iter != mObjectList.end())
{
if(*iter == object) //This is the problem section here
{
iter = mObjectList.erase(iter);
}
else
{
iter++;
}

}
}
[/CODE]

Any ideas how to get rid of this error? All of the examples I am finding makes it look like you can make comparisons this way.

Share this post


Link to post
Share on other sites
Brother Bob    10344
Have you defined an [i]operator ==[/i] to compare two [i]GraphicsObject[/i] objects?

Also, why do you define an iterator somewhere other than local to where you use it? Do you have it as a member of the GLApp class? Don't do that. Define your iterators as local variables, preferably within the smallest possible scope.

Share this post


Link to post
Share on other sites
Chrono1081    108
[quote name='Brother Bob' timestamp='1307313328' post='4819885']
Have you defined an [i]operator ==[/i] to compare two [i]GraphicsObject[/i] objects?

Also, why do you define an iterator somewhere other than local to where you use it? Do you have it as a member of the GLApp class? Don't do that. Define your iterators as local variables, preferably within the smallest possible scope.
[/quote]

Thank you for the reply. I moved my iterator into my removeElement() (my mind is fried from working on this, I have no clue why I stuck it up in the OGLApp class :P)

As for the == I didn't overload it. I didn't think I had to since the examples from the books I have don't mention overloading. I will try that next.

EDIT: SOLVED! I'm stupid. I need to walk away from this thing (and learn more about STL, I never use it hence why I'm having so many problems.

Here is what works for me:

[CODE]

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 --;
}
}
}
[/CODE]




Thank you all again SO much for your help! I really appreciate it. Also, per suggestions above I will change it to a while loop.

Share this post


Link to post
Share on other sites
Pitterbai    103
[quote name='Chrono1081' timestamp='1307314011' post='4819888']
[CODE]
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 --;
}
}
}
[/CODE]
[/quote]

In this scenario, you could overload the == operator to compare the two objects' IDs, assuming IDs are unique.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this