CODEWITHSUNDEEP
c++crashiteratorerase
user1330734
user1330734

Reputation: 470

C++ vector iterator: erase() last item crash

The following code checks if a ball overlaps with a brick. The brick is highlighted and erased from the list if the overlap occurs.

I am experiencing a core dump when erasing only the last item in the vector brickWall. Commenting the erase() line, the code seems to work well.
After looking through similar issues on forums, I believed I was iterating through and erasing the items in the vector properly, but I suspect it may not be the case.

Any suggestions on this code would be much appreciated!

void Game::updateEntities()
{
    ballMother.update();
    for (std::vector<gdf::Entity>::iterator it = brickWall.begin(); it != brickWall.end(); ++it) 
    {
        if (it->getRect().intersects(ballMother.getRect())) {
            it->rect.setFillColor(sf::Color::Red);
            it = brickWall.erase(it);
        }
        else {
            it->rect.setFillColor(it->colour);                
        }
    }
}

Upvotes: 2

Views: 4180

Answers (2)

Collin
Collin

Reputation: 12287

K-ballo found the mistake, and presented a good idiom, but you might find it easier to use a standard algorithm and the erase-remove idiom.

struct Pred {
    Pred(const BallMother& ballMother) { 
        //save refs to whatever you need for the compare 
    }

    bool operator()(const gdf::Entity& item) {
        //collision logic here, return true if you want it erased
    }
};

Now in your function:

std::vector<gdf::Entity>::iterator end_iter =
    std::remove_if(brickWall.begin(), brickWall.end(), Pred(ballMother));

brickWall.erase(end_iter, brickWall.end());

Or more succinctly:

brickWall.erase(std::remove_if(brickWall.begin(), brickWall.end(), Pred(ballMother)),
                brickWall.end());

Upvotes: 4

K-ballo
K-ballo

Reputation: 81349

The usual idiom is

for (std::vector<gdf::Entity>::iterator it = brickWall.begin(); it != brickWall.end(); /*note nothing here*/) 
{
    if (it->getRect().intersects(ballMother.getRect())) {
        it->rect.setFillColor(sf::Color::Red);
        it = brickWall.erase(it);
    }
    else {
        it->rect.setFillColor(it->colour);                
        ++it;
    }
}

otherwise you will be skipping elements, as well as eventually crash by going past the end.

Upvotes: 11

Related Questions