two vector same pointer, delete, memory loss-CodePudding

https://github.com/gameprogcpp/code/blob/master/Chapter02/Game.cpp

std::vector<Actor*> deadActors;
for (auto actor : mActors)
{
    if (actor->GetState() == Actor::EDead)
    {
        deadActors.emplace_back(actor);
    }
}

// Delete dead actors (which removes them from mActors)
for (auto actor : deadActors)
{
    delete actor;
}

how is it okay? when pointers in deadActors are deleted, their is no memory loss in mActors?

CodePudding user response：

// Delete dead actors (which removes them from mActors)

This is incorrect; deleting the Actors does not modify the original vector. It only results in dereferencing the pointers to those Actors to be undefined behaviour.

You'd need an additional step removing the pointers from the original vector for this reason.

You could simply iterate through the vector removing the dead elements in one go though:

auto writePos = std::find_if(mActor.begin(), mActor.end(), [this](Actor* actor) {return actor->GetState() == Actor::EDead; });
if (writePos != mActor.end())
{
    delete *writePos; // delete first
    // move actors and delete dead ones
    for (auto readPos = writePos   1, end = mActor.end(); readPos != end;   readPos)
    {
        if ((**readPos).GetState() == Actor::EDead)
        {
            delete *readPos;
        }
        else
        {
            // move non dead actor
            *writePos = *readPos;
              writePos;
        }
    }

    // remove unneeded space from the end
    mActor.erase(writePos, mActor.end());
}

Note that this becomes much simpler, if you use an element type for the mActor vector that manages the lifetime of the Actor object. std::unique_ptr<Actor> would do the trick. This way any element erased from the vector automatically results in the deletion of the Actor object:

std::vector<std::unique_ptr<Actor>> actors = ...;

// remove all dead actors moving the remaining actors to the front
auto removeEnd = std::remove_if(actors.begin(), actors.end(), [](std::unique_ptr<Actor> const& actor) { return actor->GetState() == Actor::EDead; });

// remove extra space
std::actors.erase(removeEnd, actors.end());

Or with C 20:

std::vector<std::unique_ptr<Actor>> actors = ...;

std::erase_if(actors.begin(), actors.end(), [](std::unique_ptr<Actor> const& actor) { return actor->GetState() == Actor::EDead; });