Is there any more efficient solution than this to remove some elements of the vector?

{
    vector<int> v{1,2,3,4,5,6,7,8,9,10};
 
    for (int i = 0; i < v.size(); i  )
    {
        if(v[i] % 2 == 0)
        {
            auto it2 = std::remove(v.begin(), v.end(), v[i]);
            v.erase(it2);
        }
    }
     
    for (auto it = v.begin(); it != v.end(); it  )
    {
        cout << *it;
    }
    return 0;
}

CodePudding user response：

std::vector::erase invalidates iterators and references at or after the point of the erase, but the main issue with the posted code is that std::remove shifts the elements in the range, so that i may already be the next element and i will skip it.

The OP could use the erase-remove idiom (once!):

auto is_even = [](auto x){ return x % 2 == 0; };
v.erase( std::remove_if(v.begin(), v.end(), is_even)
       , v.end() );

Since C 20, we can use std::erase_if:

std::erase_if(v, is_even);

CodePudding user response：

Your code is wrong. Test with this input and you will see it break: {1,2,2,2,2,1}

When you remove, all elements are shifted so that the next element is the current one. Because of that you need to either skip the increment, or decrement i when you remove any element. You also need to resize the vector to remove the tail with the "removed" items. Something like:

        if(v[i] % 2 == 0)
        {
            auto it2 = remove(v.begin(), v.end(), v[i]);
            v.erase(it2);
            v.resize(v.size()-1);
            --i;
        }

Having said that, this approach is not efficient and is very risky. You should use erase-remove idiom that "compacts" the elements matching the query to the beginning of the vector, before removing the remaining elements from the tail. This approach has the huge advantage of updating the whole vector only once.