Removing all elements from one vector that are contained in the other in C++?

Question

I have 2 vectors vc and v2 I want to remove all elements from vc that are contained in v2.I try to do this by 2 nested loops. However the compiler gives an error: Debug Assertion Failed. I would like to ask why is that and how can I fix it? Thanks in advance!

#include <iostream>
#include <vector>
#include <string>
using namespace std;
vector <string> vc;
vector <string> v2;
int main()
{
    vc.push_back("ala");
    vc.push_back("bala");
    vc.push_back("test");
    vc.push_back("sample");
    // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - 
    v2.push_back("test");
    v2.push_back("bala");
    for (auto i = vc.begin(); i != vc.end(); i++) {
        for (auto j = v2.begin(); j != v2.end(); j++) {
            if (i == j) {
                vc.erase(i);
            }
        }
    }
    //it should print only ala and sample after  the removal process, but it gives
    //debug assertion error
    for (int i = 0; i < vc.size(); i++) {
        cout << vc[i] << endl;
    }
}

Erasing item from a vector invalidates iterators being used in the for loop however comparing iterators will always fail because those iterators belong to different vectors, you should compare values instead. — user7860670, Dec 05 '18 at 07:34
Does it have to be the same vector or can you just make a new one? Currently your program crashes because `vc.erase(i)` invalidates all iterators past that point. — Qubit, Dec 05 '18 at 07:34
@Jabberwocky They obviously can be compared but only if both iterators belong to the same container. — user7860670, Dec 05 '18 at 07:48

lubgr · Answer 1 · 2018-12-05T18:40:53.050

As pointed out in the comments, you have undefined behavior in your snippet twice. First, you compare two iterators that don't refer to the same container. Second, the vc iterator and loop variable i is invalidated when vc.erase(i) is called.

Fixing this is a good example for leveraging the <algorithm> header and common idioms, as implementing such things by hand is error prone. What you need is the so called erase-remove-idiom:

#include <algorithm>

auto isInV2 = [&v2](const auto& element){
    return std::find(v2.cbegin(), v2.cend(), element) != v2.cend(); };

vc.erase(std::remove_if(vc.begin(), vc.end(), isInV2), vc.end());

Depending on the circumstances of your application, it might also be suitable to keep the vectors sorted (or sort them at some point) and then use binary search to check if an element is present, which scales better with larger sequences.

auto isInV2LogN = [&v2](const auto& element){
    return std::binary_search(v2.cbegin(), v2.cend(), element); };

// Important: v2 must be sorted, otherwise std::binary_search doesn't work:
std::sort(v2.begin(), v2.end());

vc.erase(std::remove_if(vc.begin(), vc.end(), isInV2LogN), vc.end());

please check before `erase` does `remove_if` returns `end()` because it is UB! — Gelldur, Dec 05 '18 at 10:56
@DawidDrozd I disagree, if `remove_if` returns an `end()` iterator, the half-open range `[end, end)` passed to `std::vector::erase` is empty, and from [cppreference](https://en.cppreference.com/w/cpp/container/vector/erase): "The iterator first does not need to be dereferenceable if first==last: erasing an empty range is a no-op." — lubgr, Dec 05 '18 at 18:20

score 1 · Answer 2 · answered Dec 05 '18 at 10:44

If you are allowed to sort input, you might use std::set_difference:

std::vector<std::string> vc { "ala", "bala", "test", "sample" };
std::vector<std::string> v2 { "test", "bala" };

std::sort(vc.begin(), vc.end());
std::sort(v2.begin(), v2.end());

std::vector<std::string> res;
std::set_difference(vc.begin(), vc.end(),
                    v2.begin(), v2.end(), 
                    std::back_inserter(res));

Demo

Removing all elements from one vector that are contained in the other in C++?

2 Answers2