-1

I am new to STL and was trying a simple program to insert elements using push_back and trying to remove even indexed elements.

I took n elements and pushed it into the vector. But when I erase it I either get segmentation fault or some undesired output.

  for(i=0;i<n;++i)
   {
     if(i%2==0)
       v.erase(v.begin()+i);
   }

If I use n-1 instead of n it works but does not give the desired output.

dread_user
  • 21
  • 3
  • 2
    When you delete an element, `.size()` decreases and all following elements are shifted to the left. You need to take this into account. – HolyBlackCat Sep 09 '19 at 18:24
  • 2
    As an alternative, you may want to take a look at [remove_if](https://en.cppreference.com/w/cpp/algorithm/remove) – Ted Lyngmo Sep 09 '19 at 18:27
  • 1
    Handy suggestion: [`erase` returns an iterator to the next item in the list](https://en.cppreference.com/w/cpp/container/vector/erase). If you rewrite your `for` loop to use iterators all the way through, this problem becomes a lot easier. – user4581301 Sep 09 '19 at 18:33

4 Answers4

1

You can do this 

for(i=n-1;i<=0;i--)

As mentioned in comments, earasing elements of vector decreses the vector size. By changing the for loop condition, you will start to earase even indexes from the end of vector. In this way, change in size of vector will not make problem.

rana rouzbahani
  • 41
  • 4
  • Try not. Do. Or do not. There is no try. Obsessive Yoda-quoting out of the way, I recommend explaining and possibly demonstrating how this will help the asker. – user4581301 Sep 09 '19 at 19:26
0

Just to entertain your solution

void EraseEveryOdd(std::vector<int>& v) {
  if ((v.size() % 2) > 0)
    v.pop_back();

  auto size = v.size() / 2;

  for (size_t i = 0; i < size; ++i)
    v.erase(v.begin() + i);
}

void EraseEveryEven(std::vector<int>& v) {
  if ((v.size() % 2) == 0)
    v.pop_back();

  auto size = (v.size() / 2) + 1;

  for (size_t i = 1; i < size; ++i)
    v.erase(v.begin() + i);
}
Tom Trebicky
  • 638
  • 1
  • 5
  • 11
0

By always removing elements with erase, your function will have a O(n²) runtime. A better option would be to compact the elements first, and only then erase all elements that come after the remaining ones:

#include <utility>
#include <vector>

void remove_odd_indices(std::vector<int> & inout)
{
    auto write = inout.begin();
    auto read = inout.begin();
    for(auto n = inout.size(), i = 0 * n; i < n; ++i, ++read)
    {
        if(i % 2 == 0)
            continue;
        *write++ = std::move_if_noexcept(*read);
    }
    inout.erase(write, inout.end());
}
Joe
  • 6,497
  • 4
  • 29
  • 55
0

Keep the counting in mind, just initialize the itr to correct position after erase, same itr points to next undeleted element in vector

vector<int> vec = { 1,2,3,4,5,6,7,8,9,10};

int main()
{

        auto itr=vec.begin();

        while( itr != vec.end() ){
                itr++;
                vec.erase(itr);
        }
        for(auto data : vec) cout << " " << data << " " ;
        cout << endl;
}
Vivek Saxena
  • 49
  • 2