Erase all even numbers from vector using find_if

Question

#include <iostream>
#include <vector>
#include <algorithm>
#include <time.h>
#include <iomanip>

using namespace std;

bool isEven(int n)
{ 
    return n%2 == 0;
}

int main()
{
    srand(time(NULL));

    vector<int> myVec;

    for(int i = 0; i < 20; i++)
    {
        myVec.push_back(rand() % 100);
    }   
    while(1)
    {   
          vector<int>::iterator q = std::find_if(myVec.begin(), myVec.end(), isEven);
          cout << *q << endl;
          if(q == myVec.end())
          {   
             myVec.erase(q);
             break;
          }   
          else
             myVec.erase(q);        
      }

    return 0;
}

This code is giving segmentation fault. The above code is to remove all the even numbers from the vector using find_if and erase function

Please help. Any help will be highly appreciated.

EDIT: I have edited it to make sure that iterator will be valid always.

Still it is giving segmentation fault.

You are erasing an end iterator `q` in the first `if` branch. — Human-Compiler, Dec 14 '17 at 17:00
The point of the comments and answers is that writing loops is not necessary to do this work. If the job sounds like something that has been done millions of times before, then more than likely there is an algorithm to do the job, and that algorithm is `std::remove_if`. — PaulMcKenzie, Dec 14 '17 at 17:01

StoryTeller - Unslander Monica · Answer 1 · 2017-12-14T16:56:37.990

std::vector::erase invalidates all iterators to and after the point of erasure. You can't continue using that iterator, not to increment it, use it to access the vector, or even compare it to end().

The correct algorithm to use is std:remove_if. Unlike the name implies, it will only move all even items of the vector "to the back", without invalidating any iterators. It will return an iterator to the start of this sub-range, which you can then just feed to the appropriate erase overload (the one that accepts a pair of iterators).

This has been used so much in code that it's even named "the erase-remove idiom".

peti446 · Accepted Answer · 2017-12-14T17:04:27.110

When using the erase(it); function the iterator changes so you need to set the iterator again to the new one returned by the erase function.

In your code, you are checking for the end if(q == myVec.end()) and then using erase this will throw an error as.end() does not point to data, and to be able to erase an item from the vector the iterator needs to be valid. So by changing if(q == myVec.end()) to if(q == (myVec.end()-1)) it will allow you to delete the last element in case of been a pair.

Erase all even numbers from vector using find_if

2 Answers2