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

Някои предположения/дефиниции:

• 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. Ако изберете втория подход, не забравяйте да актуализирате този цикъл, за да проверите за nullptrs в 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 обекта.

Все още трябва да внимавате за erase невалидни итератори, така че продължете да използвате идиома за изтриване-премахване, както е обяснено от Tyler McHenry. Например:

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

Забележка - auto&& е разрешено тук от C++14; в C++11 ще трябва да използвате std::unique_ptr<Object>&. Задължителните включвания са <algorithm> и <memory>.

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