dcjboi
dcjboi

Reputation: 31

Is it possible to remove elements from a vector of shared_ptr?

Say I have

vector<shared_ptr<string>> enemy;

how do I remove elements from the enemy vector?

Thanks for your help in advance

**Edit (code in context)

void RemoveEnemy( vector<shared_ptr<Enemy>> & chart, string id )
{
  int i = 0;
  bool found = FALSE;
  for(auto it = chart.begin(); it != chart.end(); i++)
  {
    if(id == chart[i]->GetEnemyID() )
    {
        found = TRUE;
        chart.erase(it);
    }
}

the code above segfaults me

Upvotes: 1

Views: 1826

Answers (3)

Zan Lynx
Zan Lynx

Reputation: 54325

I like mine which will remove aliens at high speed and without any care for the ordering of the other items. Removal with prejudice!

Note: remove_if is most often used with erase and it will preserve the order of the remaining elements. However, partition does not care about the ordering of elements and is much faster.

partition-test.cpp:
make partition-test && echo 1 alien 9 alien 2 8 alien 4 7 alien 5 3 | ./partition-test

#include <algorithm>
#include <iostream>
#include <iterator>
#include <memory>
#include <string>
#include <vector>

using namespace std;

template <typename T>
ostream &operator<<(ostream &os, const vector<T> &container) {
  bool comma = false;
  for (const auto &x : container) {
    if (comma)
      os << ", ";
    os << *x;
    comma = true;
  }
  return os;
}

int main() {
  vector<shared_ptr<string>> iv;
  auto x = make_shared<string>();
  while (cin >> *x) {
    iv.push_back(x);
    x = make_shared<string>();
  }
  cout << iv << '\n';

  iv.erase(partition(begin(iv), end(iv),
                     [](const auto &x) { return *x != "alien"s; }),
           end(iv));
  cout << iv << '\n';
  return 0;
}

Upvotes: 0

Zan Lynx
Zan Lynx

Reputation: 54325

The problem with your code is that erase() invalidates the iterator. You must use it = chart.erase(it).

Upvotes: 2

Remy Lebeau
Remy Lebeau

Reputation: 595712

You remove elements the same way you remove any elements from any std::vector - via the std::vector::erase() method, for instance. All you need for that is an iterator to the desired element to remove.

In your case, since you are storing std::shared_ptr<std::string> objects rather than storing actual std::string objects, you may need to use something like std::find_if() to find the vector element containing the desired string value, eg:

void removeEnemy(string name)
{
    auto iter = std::find_if(enemy.begin(), enemy.end(),
        [&](auto &s){ return (*s == name); }
    );
    if (iter != enemy.end())
        enemy.erase(iter);
}

UPDATE: in the new code you have added, you are incorrectly mixing indexes and iterators together. You are creating an infinite loop if the vector is not empty, as you never increment the it iterator that controls your loop, you are incrementing your index i variable instead (see what happens when you don't give your variables unique and meaningful names?). So you end up going out of bounds of the vector into surrounding memory. That is why you get the segfault error.

Even though you are (trying to) use an iterator to loop through the vector, you are using indexes to access the elements, instead of dereferencing the iterator to access the elements. You don't need to use indexes at all in this situation, the iterator alone will suffice.

Try this instead:

void RemoveEnemy( vector<shared_ptr<Enemy>> & chart, string id )
{
  for(auto it = chart.begin(); it != chart.end(); ++it)
  {
    if (id == it->GetEnemyID() )
    {
      chart.erase(it);
      return;
    }
}

Or, using the kind of code I suggested earlier:

void RemoveEnemy( vector<shared_ptr<Enemy>> & chart, string id )
{
    auto iter = std::find_if(chart.begin(), chart.end(),
        [&](auto &enemy){ return (enemy->GetEnemyID() == id); }
    );
    if (iter != chart.end())
        chart.erase(iter);
}

Upvotes: 2

Related Questions