CODEWITHSUNDEEP
c++vectorvisual-studio-2015erase
CraftedGaming
CraftedGaming

Reputation: 529

C++ Deleting an object in a vector

I am currently using a vector to hold the people in a program. I am trying to delete it with

vectorname.erase(index);

I pass the vector in a function, as well as the element that I want to delete. My main problem is how can I improve my code in terms of compilation speed?

#include <iostream>
#include <string>
#include <vector>
using namespace std;

class person {
    private:
        string name;
    public:
        person() {}
        person(string n):name(n){}
        const string GetName() {return name;}
        void SetName(string a) { name = a; }
};

void DeleteFromVector(vector<person>& listOfPeople,person target) {
    for (vector<person>::iterator it = listOfPeople.begin();it != listOfPeople.end();++it) {//Error 2-4
        if (it->GetName() == target.GetName()) {
            listOfPeople.erase(it);
            break;
        }
    }
}

int main(){
    //first group of people
    person player("Player"), assistant("Assistant"), janitor("Janitor"), old_professor("Old Professor");

    //init of vector
    vector<person> listOfPeople = { player, assistant, janitor, old_professor };

    DeleteFromVector(listOfPeople, janitor);
}

Upvotes: 2

Views: 9793

Answers (3)

djacob
djacob

Reputation: 120

listOfPeople.erase(
                   remove(listOfPeople(), listOfPeople.end(), target),
                   listOfPeople.end()
                  )

"remove" operation in this erase-remove idiom will move all the elements except target to the front of the vector range and "erase" operation will delete all elements that are at the end that satisfies the target criteria. This is much efficient even though it is not as expressive as iterative version.

Upvotes: 1

PaulMcKenzie
PaulMcKenzie

Reputation: 35440

You don't need that loop to delete the object from the vector. Just use std::find_if:

#include <algorithm>
//...
void DeleteFromVector(vector<person>& listOfPeople, const person& target) 
{
    // find the element
    auto iter = std::find_if(listOfPeople.begin(), listOfPeople.end(),
                             [&](const person& p){return p.GetName() == target.GetName();});

    // if found, erase it
    if ( iter != listOfPeople.end())
       listOfPeople.erase(iter);
}

Upvotes: 3

zhm
zhm

Reputation: 3641

There is no need to define index, iterator can be used to access objects in vector:

for (vector<person>::iterator it = listOfPeople.begin(); it != listOfPeople.end(); ++it) {//Error 2-4
    if (it->GetName() == target.GetName()) {
        listOfPeople.erase(it);
        break;
    }
}

Since next line is to break for loop, we don't consider invalid iterator problem here.

Upvotes: 7

Related Questions