Deleting objects in vector C++

Question

I have the following problem:

    vector<CPerson>toRemove;// has some objects that are contained in m_vObjects
    vector<CPerson>m_vObjects;

    for (auto it = toRemove.begin(); it != toRemove.end(); ++it)
    {
        for (auto iter = m_vObjects.begin(); iter != m_vObjects.end(); ++iter)
        {
            iter = m_vObjects.erase(it);
        }

    }

What I want to do is delete all the objects from m_vObjects that are contained in toRemove. I have tried a lot of stuff, but nothing worked fine.

Possible duplicate of [Erasing elements from a vector](https://stackoverflow.com/questions/347441/erasing-elements-from-a-vector) — CinCout, Dec 15 '17 at 08:20

Innokentiy Alaytsev · Answer 1 · 2017-12-15T08:38:36.860

2

Your code is wrong: you cannot use iterator from one container to remove items from the other container. One way to achieve what you want is to use std::set_difference (requires your arrays to be sorted):

vector<CPerson>toRemove;// has some objects that are contained in m_vObjects
vector<CPerson>m_vObjects;

vector<int> diff;

std::set_difference(
    m_vObjects.begin(), m_vObjects.end(), 
    v2.begin(), v2.end(), 
    std::inserter(diff, diff.begin()));

m_objects = diff;

If it is not desired to sort arrays, then you can use std::remove:

for (const auto& element_to_remove : toRemove) {
    std::remove (
        m_vObjects.begin (),
        m_vObjects.end (),
        element_to_remove);
}

edited Dec 15 '17 at 08:38

answered Dec 15 '17 at 08:17

Innokentiy Alaytsev

842
8
20

4

You forgot to mention, that `std::set_difference` works on **sorted** ranges. So, if the vectors, that OP uses, are not sorted, your suggestion invokes UB. – Algirdas Preidžius Dec 15 '17 at 08:20
@AlgirdasPreidžius, you are right, I shall fix my answer. – Innokentiy Alaytsev Dec 15 '17 at 08:34

Nikita Smirnov · Answer 2 · 2017-12-15T08:37:38.623

0

1) You cannot use iterator from toRemove in m_vObjects. They are strongly connected to each instance of collection.

2) It is not really a good practice to use loop on iterators while modifying collection: for example you have deleted an object after current interator, but he still thinks that object is there. If I'm correct, it causes undefined behavior.

3) The fix should be check equality of objects, that are pointed by iterators.

  if( ( *it == *iter )
  {
     iter= vObjects.erase( iter );
  }

But still, my solution works only if == is overloaded for person, or you have another method that checks equality. And it is very bad for performance. Actually, IMHO, such actions at vectors are always bad for performance, it's better to use set or map if you need to find something. And set has some in-build methods to intersect them.

edited Dec 15 '17 at 08:37

answered Dec 15 '17 at 08:27

Nikita Smirnov

813
8
17

1

Your "solution" invokes UB, because `innerIt` is invalidated at `vObjects.erase( innerIt );` call. – Algirdas Preidžius Dec 15 '17 at 08:30
@AlgirdasPreidžius lol, yes, I explained that it's bad and used it myself. Thanks that payed attention, fixed – Nikita Smirnov Dec 15 '17 at 08:31
1

1) Hint: [`std::vector::erase`](http://en.cppreference.com/w/cpp/container/vector/erase) Returns "_Iterator following the last removed element._". 2) If `m_vObjects` contains multiple identical objects - your updated solution won't remove them all. – Algirdas Preidžius Dec 15 '17 at 08:36
@AlgirdasPreidžius I knew about problem with multiple object, but before your hint had no ideas how to deal with it. – Nikita Smirnov Dec 15 '17 at 08:38

Deleting objects in vector C++

2 Answers2