Как удалить указатель объекта из вектора, не вызывая ошибки памяти?

Некоторые предположения/определения:

• objects переменная-член, что-то вроде vector<Object*> objects;
• p также является переменной-членом, что-то вроде vector<Object*>::iterator p;

Итак, p — это итератор, *p — это указатель на объект, а **p — это объект.

Проблема в том, что этот метод:

void Engine::killUpdate(std::string command) {
  if ((*p)->getIsDead() == true) {
    delete *p;
  }
}

освобождает объект, на который указывает *p, указатель в векторе в позиции, на которую ссылается итератор p. Однако сам указатель *p все еще находится в векторе, теперь он просто указывает на память, которая больше не выделена. В следующий раз, когда вы попытаетесь использовать этот указатель, вы вызовете неопределенное поведение и, скорее всего, сбой.

Поэтому вам нужно удалить этот указатель из вашего вектора после того, как вы удалили объект, на который он указывает. Это может быть таким простым, как:

void Engine::killUpdate(std::string command) {
  if ((*p)->getIsDead() == true) {
    delete *p;
    objects.erase(p);
  }
}

Однако вы вызываете killUpdate из update в цикле, который перебирает вектор objects. Если вы воспользуетесь приведенным выше кодом, у вас возникнет другая проблема: как только вы сотрете p из вектора objects, выполнение p++ в операторе цикла for станет небезопасным, потому что p больше не является допустимым итератором.

К счастью, STL предоставляет очень хороший способ обойти это. vector::erase возвращает следующий действительный итератор после того, который вы стерли! Таким образом, вы можете обновить метод killUpdate p вместо оператора for-loop, например.

void Engine::update(string command) {
  if (getGameOver() == false) {
    for (p = objects.begin(); p != objects.end(); /* NOTHING HERE */) {
      // ...
      killUpdate(command);
    }
  }
}

void Engine::killUpdate(std::string command) {
  if ((*p)->getIsDead() == true) {
    delete *p;
    p = objects.erase(p);
  } else {
    p++;
  }
}

Это, конечно, предполагает, что вы всегда вызываете killUpdate в цикле, но я уверен, что вы можете найти способ обойти это, если вы этого не сделаете — просто выполните p++ в конце for- тело цикла в случае, если вы не вызвали killUpdate.

Также обратите внимание, что это не особенно эффективно, поскольку каждый раз, когда вы стираете элемент вектора, элементы, следующие за ним, должны сдвигаться назад, чтобы заполнить пустое пространство. Так что это будет медленно, если ваш вектор objects большой. Если вместо этого вы использовали std::list (или уже используете его), то это не проблема, но у списков есть и другие недостатки.

Вторичный подход заключается в том, чтобы перезаписать каждый указатель на удаленный объект с помощью nullptr, а затем использовать std::remove_if для удаления их всех за один раз в конце цикла. Например.:

void Engine::update(string command) {
  if (getGameOver() == false) {
    for (p = objects.begin(); p != objects.end(); p++) {
      // ...
      killUpdate(command);
    }
  }
  std::erase(std::remove_if(objects.begin(), objects.end(), 
                            [](const Object* o) { return o == nullptr; }), 
             objects.end());
}

void Engine::killUpdate(std::string command) {
  if ((*p)->getIsDead() == true) {
    delete *p;
    *p = nullptr;
  } 
}

На этот раз предполагается, что у вас никогда не будет элемента nullptr из objects, который вы хотите сохранить по какой-то причине.

Поскольку вы, кажется, новичок, я должен отметить, что это:

  std::erase(std::remove_if(objects.begin(), objects.end(), 
                            [](const Object* o) { return o == nullptr; }),
             objects.end());

это идиома стереть-удалить, которая хорошо объясняется в Википедии. Он стирает элементы из вектора, если они возвращают true, когда для них вызывается данный объект-функция. В этом случае объект функции:

[](const Object* o) { return o == nullptr; }

Который представляет собой лямбда-выражение и, по сути, является сокращением для экземпляра объект с этим типом:

class IsNull {
 public:
   bool operator() (const Object* o) const {
     return o == nullptr;
   }
};

Последнее предостережение относительно второго подхода. Я только что заметил, что у вас есть еще один цикл поверх objects в scrollUpdate. Если вы выберете второй подход, обязательно обновите этот цикл, чтобы проверить наличие nullptr в objects и пропустить их.

Вот проблема (отформатирована для удобочитаемости):

void Engine::update(string command)
{
    if (getGameOver()==false)
    {
        for (p=objects.begin();p!=objects.end();p++)
        {
            spawnUpdate(command);  // changes vector
//...
       }
    }
//...
}

void Engine::spawnUpdate(string command)
{
//...
    objects.push_back(new Bullet((*p)->getPosX(),(*p)->getPosY(),(*p)->getState())); // no
//...
}

У вас есть цикл с итератором p, который указывает на элементы в векторе object. Когда вы вызываете objects.push_back, итератор для вектора может стать недействительным. Таким образом, этот итератор цикла p больше не годится. Увеличение его в for() приведет к неопределенному поведению.

Один из способов обойти это — создать временный вектор, в котором будут храниться ваши обновления. Затем вы добавляете обновления в конце обработки:

void Engine::update(string command)
{
    std::vector<Object*> subVector;
    if (getGameOver()==false)
    {
        for (p=objects.begin();p!=objects.end();p++)
        {
            spawnUpdate(command, subVector);
            //...
        }
    }

    // add elements to the vector
    object.insert(object.end(), subVector.begin(), subVector.end());
}

void Engine::spawnUpdate(string command, std::vector<Object*>& subV)
{
    if (command.compare("shoot")==0)
    {
        cout<<"Bang!"<<endl;
        if ((*p)->getType() == PLAYER)
            subV.push_back(new Bullet((*p)->getPosX(),(*p)->getPosY(),(*p)->getState()));
        cout<<"Bullet success "<<endl;
    }
}

Вы можете избежать большинства этих проблем, не используя необработанные указатели. Очевидно, что ваш код использует семантику, согласно которой вектор владеет указателями, поэтому вы можете выразить это напрямую:

std::vector< std::unique_ptr<Object> > objects;

Затем вы можете вставить в вектор с помощью objects.emplace_back(arguments,to,Object,constructor); , и когда вы удалите из вектора, он автоматически delete Object.

Вам по-прежнему нужно следить за тем, чтобы итераторы erase недействительны, поэтому продолжайте использовать идиому стереть-удалить, как объяснил Тайлер МакГенри. Например:

objects.erase( std::remove_if( begin(objects), end(objects),
    [](auto&& o) { return o->getIsDead(); }),  end(objects) );

Примечание. auto&& разрешено здесь, начиная с C++14; в С++ 11 вам придется использовать std::unique_ptr<Object>&. Обязательными являются <algorithm> и <memory>.

И, пожалуйста, прекратите использовать глобальные итераторы, оставьте p локальным для функции и передавайте любые аргументы, которые вам нужно передать.