CODEWITHSUNDEEP
c++vector
Steven K.
Steven K.

Reputation: 11

Cannot erase item by index from C++ vector

I'm trying to learn vectors in C++, so I write a very basic code. Basically, I have a vector which holds pointer to person structure. Which is like this:

typedef struct _person
{
    unsigned long id;
    char* name;
    unsigned long age;
}person;

Then, I've a vector which is defined like this:

vector<person*> MyPersons;

Then, for testing I've created some person structure and push them to vector via push_back function. Now, for example I want to remove first person that has age 25 from my vector. So, what I do is search in vector and get index of that person, then erase that index from vector. 

for(size_t i = 0; i < MyPersons.size(); i++)
{
    pperson = MyPersons.at(i);
    if(pperson->age == 25)
    {
        index = i;
        break;
    }
}

MyPersons.erase(MyPersons.begin() + index )

Now, it should be remove that item from list, right? But, instead, it gives me error:

Debug Assertion Failed!

Expression: vector erase iterator outside range

But this is impossible. I've debug my code with Visual Studio, and I see that index value is valid. MyPersons size is 5 and index is 2.

Any help very appreciated. Sincerely

Upvotes: 1

Views: 301

Answers (2)

Daniel
Daniel

Reputation: 8451

The problem is likely that index is initialised to a value not in the range of MyPersons, so if the value isn't found then you get the error.

You should use standard algorithms for these type of task, to delete the first person* with age 25 you could use:

auto itr = std::find_if(MyPersons.cbegin(), MyPersons.cend(),
                       [] (person* pperson) { return pperson->age == 25; });
if (itr != MyPersons.cend()) MyPersons.erase(itr);

To delete all person*s with age 25 you could use:

MyPersons.erase(std::remove_if(MyPersons.begin(), MyPersons.end(),
                [] (person* pperson) { return pperson->age == 25; }),
                MyPersons.end());

Using standard algorithms is preferable than using hand-rolled loops. The intention is clearer, the resulting code is often less verbose, and most importantantly, standard algorithms more likely to be correct.

Also, you're probably going about things the wrong way storing pointers to your data in the std::vector.

Upvotes: 4

Caleth
Caleth

Reputation: 63152

You don't need the typedef struct _person preamble, that's a C-ism.

struct person
{
    unsigned long id;
    std::string name;
    unsigned long age;
}

You also don't need to new objects, they can be values

std::vector<person> people;

To remove the first person aged 25:

auto it = std::find_if(people.begin(), people.end(), [](person & p) { return p.age = 25; });
if (it != people.end()) people.erase(it);

To remove all people aged 25:

auto it = std::remove_if(people.begin(), people.end(), [](person & p) { return p.age = 25; });
people.erase(it, people.end());

Upvotes: 4

Related Questions