Removal of an Object from a List during iteration-CodePudding

I am trying to make a list of projectile objects that are on screen which execute different methods. This has worked but I keep getting an error when I try to remove the object once it is off screen.

std::list<Bullet> bullets;
//bullets are added on mouse click

//start iterating through objects to move
std::list<Bullet>::iterator mover;
for (mover = bullets.begin(); mover != bullets.end(); mover  ){
    //Checks if bullet is off screen
    if (mover->x < 0 || mover->x > 800 || mover->y < 0 || mover->y > 800) {
        /*
          The deconstructor works, but when bullets.erase(mover); is done, a hard exception occurs.
          I believe it may be due to it iterating to a place with nothing, but I am unsure,
          and more unsure on how to fix it
        */
        mover->~Bullet();
        bullets.erase(mover);
        std::cout << "Kablooey" << std::endl;
        //an attempted fix that doesn't seem to do anything
        if (mover == bullets.end()) {
            break;
        }
    }
    //makes current object move afterwards, could also be source of error?? 
    //Should I make it check if there is anything there first?
    mover->Move();
    std::cout << "nyoom" << std::endl;
}

The error says as follows: "Expression: List iterators incompatible".

What should I do to make the object be deleted from the list without an error?

CodePudding user response：

Your code has multiple sources of undefined behavior:

erase() invalidates the iterator that is passed to it. You are dereferencing and incrementing that iterator after it has been invalidated. erase() returns a new iterator to the next item in the list, you need to continue your loop using that iterator.
you are manually destroying objects that you have no business destroying. The list owns the objects and will destroy them when they are erased, or when the list itself is destroyed.

Try this instead:

std::list<Bullet> bullets;
//bullets are added on mouse click

...

//start iterating through objects to move
std::list<Bullet>::iterator mover = bullets.begin();
while (mover != bullets.end()){
    //Checks if bullet is off screen
    if (mover->x < 0 || mover->x > 800 || mover->y < 0 || mover->y > 800) {
        mover = bullets.erase(mover);
        std::cout << "Kablooey" << std::endl;
    } else {
        mover->Move();
        std::cout << "nyoom" << std::endl;
          mover;
   }
}

If you can move std::cout << "Kablooey" << std::endl; into the ~Bullet() destructor, you can then replace the erase loop with a standard algorithm, eg:

std::list<Bullet> bullets;
//bullets are added on mouse click

...

bullets.erase(
    std::remove_if(bullets.begin(), bullets.end(),
        [](const Bullet &b){ return (b.x < 0 || b.x > 800 || b.y < 0 || b.y > 800); },
    bullets.end()
);

/* Alternatively, in C  20:
std::erase_if(bullets,
    [](const Bullet &b){ return (b.x < 0 || b.x > 800 || b.y < 0 || b.y > 800); }
);
*/

for(auto &b : bullets) {
    b.Move();
    std::cout << "nyoom" << std::endl;
}