Thread: Delete objects from a vector

So, I have created my own containerclass that holds a vector. Im using it in a game and when certain objects hits a wall I want them to be erased. Here is the code:

Code:

void Game::aliveTest()
{
    for (std::vector<Entity*>::iterator i = container.begin(); i!=container.end(); i++)
    {
        if ((*i)->getType() == 8)
        {
            if (((Projectile*)*i)->isAlive()==false)
            {
                delete (*i);
                i=container.erase(i);
            }
        }
    }
}

When i try to do this I get a compile error on the "=" saying that: "no operator "=" matches these operands". If I remove the "i=" the program crashes and I get "vector iterator not incrementable".

Do I need to specify my own "= operator" in my containerclass or what's the problem?

Originally Posted by hk_mp5kpdw

You definitely need the i= part otherwise you'd be operating on an invalid iterator after that point. However, there should not be anything wrong with that particular assignment statement... at least from what I can see. Maybe I'm missing something that someone else can see?

[edit]
Is the error from the container.erase(i) line or is it on the container.begin() line? If i does not match what container.begin() returns that could be where the error is coming from.
[/edit]

---

One problem with such a set up... when you return the iterator (to the next element in the vector) after you erase it, your loop then goes on to increment the iterator which skips the check of the vector element after the one you just removed. It's almost as if you were incrementing twice. This means you could be missing out on the deletion of some objects. You only should increment i if there was no deletion taking place since when a delete does happen the increment is effectively taken care of for you by the return value of the erase call.

Let's say you have a list of numbers and you loop through removing even ones. Starting with:

2->4->5

You would remove the 2 and the return value of the erase function would put i at 4. Then, towards the end of the for loop, you increment i again and i now points to 5 the next time through the loop. Well you've just missed out on removing 4 from the list because of this.

The error is from the line "i=container.erase(i);" hence the red =

The thing is that there isn't anything suspicious about that line from what you showed. Usually when

"no operator "=" matches these operands".

shows up there is one value attempting to be stored in a variable of a completely different, non-compatible type. Meaning in this case that the i isn't the same type as the returned value, and there is no way to convert the return value into i's type. That doesn't make any sense given that you passed in the original i.

Could you post exactly what the compiler output (the line number and excetera)?

Error 1 error C2679: binary '=' : no operator found which takes a right-hand operand of type 'void' (or there is no acceptable conversion)

Yeah, I'm becoming more convinced that the line where you think the problem is, isn't the problem at all. Please make certain that the error refers to that line in that source file.

The complete signature for erase is:

Code:

 iterator erase ( iterator position );

The only conclusion I can reach is that you aren't showing us what's wrong.

It's indeed the line Im refering to that is the right one. But I think I know what the problem is now. I had to make my own erase() function in the container class nad the return type is void. Since I use "i=" Im guessing the return type has to be something else then void. I don't know what the return type should be

Ok, so I rewrote the erase function so now it looks like this:

Code:

std::vector<Entity*>::iterator Container::erase(std::vector<Entity*>::iterator i){
    mEntity.erase(i);
    return i;
}

But now when I run the program, it crahses and says that: "vector iterator not incrementable"

Originally Posted by laserlight

It probably should be:

Code:

std::vector<Entity*>::iterator Container::erase(std::vector<Entity*>::iterator i){
    return mEntity.erase(i);
}

By the way, if you're going to write this Container class with iterators, then you should give it its own iterator type. This can be done by a typedef of std::vector<Entity*>::iterator, so you can write:

Code:

Container::iterator Container::erase(Container::iterator i) {
    return mEntity.erase(i);
}

Otherwise, if a user of this Container class relies on std::vector<Entity*>::iterator, and some day the maintainer of the Container class decides that mEntity really should be a std::deque<Entity*> instead, then we have a problem.

Thanks a lot for the help! Now it works as intended. As for your suggestion, it's not really anything that's neccesary since this is a personal game that Im making, but I'll think about changing it

I just want to point out that with the vector<Entity*>::iterator being the type of iterator, I assumed wrongly that container was also a vector<Entity*>, which it wasn't. So, typedefs can make code clearer ...for idiots like me.