CODEWITHSUNDEEP
c++vectoriteratorerase
Joel Seah
Joel Seah

Reputation: 714

Segmentation fault when erasing last element in vector

I'm trying to erase the last element in the vector using iterator. But I'm getting segmentation fault when erasing the element.

Below is my code:

    for (vector<AccDetails>::iterator itr = accDetails.begin(); itr != accDetails.end(); ++itr) {
    if (username == itr->username) {
            itr = accDetails.erase(itr);
    }
}

Is there something wrong with my iteration?

Upvotes: 1

Views: 1585

Answers (2)

Jerry Coffin
Jerry Coffin

Reputation: 490108

This is a good place to apply the remove/erase idiom:

accDetails.erase(
    std::remove_if(
        accDetails.begin(), accDetails.end(), 
        [username](AccDetails const &a) { return username == a.username; }),
     accDetails.end());

As a bonus, this is likely to be a little bit faster than what you were doing (or maybe quite a bit faster, if your vector is large). Erasing each item individually ends up as O(N2), but this will be O(N), which can be pretty significant when/if N gets large.

If you can't use C++11, the lambda won't work, so you'll need to encode that comparison separately:

class by_username { 
    std::string u;
public:
    by_username(std::string const &u) : u(u) {}
    bool operator()(AccDetails const &a) { 
        return u == a.username;
    }
};

accDetails.erase(
    std::remove_if(accDetails.begin(), accDetails.end(), by_username(username)), 
    accDetails.end());

Alternatively, you can overload operator== for your AccDetails class, and handle the comparison there. For example:

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

class AccDetail {
    std::string name;
    int other_stuff;
public:
    AccDetail(std::string const &a, int b) : name(a), other_stuff(b) {}

    bool operator==(std::string const &b) {
        return name == b;
    }

    friend std::ostream &operator<<(std::ostream &os, AccDetail const &a) {
        return os << a.name << ", " << a.other_stuff;
    }
};

int main(){
    std::vector<AccDetail> ad = { {"Jerry", 1}, { "Joe", 2 }, { "Bill", 3 } };

    std::cout << "Before Erase:\n";
    std::copy(ad.begin(), ad.end(), std::ostream_iterator<AccDetail>(std::cout, "\n"));
    ad.erase(
        std::remove(ad.begin(), ad.end(), "Joe"),
        ad.end());

    std::cout << "\nAfter Erasing Joe:\n";
    std::copy(ad.begin(), ad.end(), std::ostream_iterator<AccDetail>(std::cout, "\n"));
}

Upvotes: 5

alexis
alexis

Reputation: 569

I learn a safe way to erase elements from my leader. Firstly, find all the elements. Secondly, erase them one by one.

queue< vector<AccDetails>::iterator > q;
for (vector<AccDetails>::iterator itr = accDetails.begin(); itr != accDetails.end(); ++itr) {
    if (username == itr->username) {
        //itr = accDetails.erase(itr);
        q.push(itr);
    }
}
while(!q.empty()){
    vector<AccDetails>::iterator itr = q.front();
    accDetails.erase(itr);
    q.pop();
}

Upvotes: -2

Related Questions