What i am doing wrong in erasing some vector elements?

Question

Here i am trying to print the frequency of each word in the sentence, which is stored in the vector of string

void display_by_word (vector<string> vs) //pass by value is necessary because we need to delete the elements.
{
vector<string> :: size_type vec_size, i;
string to_cmp = vs[0];
int occ = 0;

for ( i = 0; i < vs.size(); ++i){

    vector <string> :: iterator it = vs.begin() + 1;
    occ = 1;

    for ( it ; it != vs.end(); ++it){

        if ( vs[i] == *it){

            vs.erase(it);
            occ++;
        }
    }

    cout << vs[i] << "     " << occ << endl;
}

}

Sometimes it works fine but sometimes it crashes.what is wrong?

Take a look at the [erase-remove idiom](https://en.wikipedia.org/wiki/Erase%E2%80%93remove_idiom). — Sergey Kalinichenko, Jul 26 '15 at 15:44
@paper.plane what good it will do for me as i am still incrementing the iterator? — OldSchool, Jul 26 '15 at 15:50

score 3 · Answer 1 · answered Jul 26 '15 at 16:19

See http://en.cppreference.com/w/cpp/container/vector/erase

Invalidates iterators and references at or after the point of the erase [...]

After the erase has happened, you cannot reuse it because it has been invalidated. It's undefined behaviour, which can include random crashes.

However, erase returns an iterator to the element following the erased one, or to end() if it was the last element, which is why the solution with it = vs.erase(it); works.

Alternatively, consider using std::remove_if, followed by the two-argument erase, which is known as the Erase-Remove Idiom. It may turn out to be more elegant and more readable than a hand-written loop. Or just rewrite the whole function to use std::count_if.

score 2 · Accepted Answer · answered Jul 26 '15 at 15:56

2

You may want to rewrite the loop as something like this

while(it != vs.end())
{
    if ( vs[i] == *it){

        it = vs.erase(it);
        occ++;
    }
    else
        it++;
}

answered Jul 26 '15 at 15:56

paper.plane

1,201
10
17

when we erase any element is it necessary that every time new memory would be allocated for the remaining vector or adjustment can be made at the current memory? – OldSchool Jul 26 '15 at 16:06
You should prefer `++it;`. http://stackoverflow.com/questions/24901/is-there-a-performance-difference-between-i-and-i-in-c – Beta Carotin Jul 26 '15 at 16:14

score 1 · Answer 3 · answered Jul 28 '15 at 09:05

1

You may should do as below: ` for ( it ; it != vs.end(); ){

    if ( vs[i] == *it){

        it = vs.erase(it);
        occ++;
    }
    else{
        ++it;
    }
}

`

answered Jul 28 '15 at 09:05

JerryYoung

267
1
3

What i am doing wrong in erasing some vector elements?

3 Answers3