c++ How to erase object using vector with auto while inside loop

Question

Hi folks at stackoverflow,

I've been wondering if there were any easy means for: iterator controlled ranged for-loops to properly erase objects from within its containers while accessing it; using auto it.

For a standard indexed controlled for-loop, I would do something like this:

void del(int i){
    cout<<"Deleting: "<<myObjects[i]<<':'<<myObjects[i]->c<<endl;
    for(unsigned int j = 0; j < allObjects.size();){
        if(allObjects[j] == myObjects[i]){
            delete allObjects[j];
            allObjects[j] = 0;
            allObjects.erase(allObjects.begin()+j);
            break;//Found and deleted first instance, escaping for-loop.
        }else{
            ++j;
        }
    }
    myObjects[i]=0;
    myObjects.erase(myObjects.begin()+i);
}

An auto for loop would look something like this:

void del(int i){
    cout<<myObjects[i]<<endl;
    for(auto it: allObjects)
        if(it == myObjects[i]){
            delete it;
            it = 0;
            //allObjects.erase();// Found, needs erasing.
        }
    myObjects[i]=0;
    myObjects.erase(myObjects.begin()+i);
}

I haven't been able to properly work this out and have been resorting to old school indexing(many ways to do it with an index).

I can delete it, and set it to 0, but how would I also erase it from the vector and possibly while in the vector without knowing an index? I know I can keep track of the loop and do so using a counter, but that would defeat the purpose of using a nice clean iterator loop.

If not to be removed in the vector, how would I go about so afterwards in an easily manner other than re-accessing the vector?

Nothing wrong with using index driven for-loops, just wanted to know if there was an easy alternative using our new friend "auto it".

Thanks.

What is your goal? Erasing elements from vectors is a pretty common thing, so make sure you read and understand all those questions automatically marked as "related" on the right side. Further, don't think that because you have a hammer (`auto`) you have to use it. — Ulrich Eckhardt, Mar 05 '16 at 11:17
In your second code snippet, it is not an iterator but refers to myObjects[/* corresponding index */]... might be misleading... range-based for loops do not provide iterators — IceFire, Mar 05 '16 at 11:18

score 6 · Accepted Answer · edited May 23 '17 at 11:45

6

I am sorry, but there is no way to use range-based for loops to erase. You should use the standard way:

for(auto it = allObjects.begin(); it != allObjects.end();)
{
    if(/* condition */)
        it = allObjects.erase(it);
    else
        ++it;
}

See also top answer here: Can we erase the items in range-based for loop in c++11

edited May 23 '17 at 11:45

Community

1
1

answered Mar 05 '16 at 11:16

IceFire

4,016
2
31
51

Alright, thanks. I figured there was some fancy way of doing it other than how we've always done it. – doingit Mar 05 '16 at 11:22
I don't think this is what you want. In case an object gets deleted, you skip one object. Because the iterator gets incremented every time. – Jens Mar 05 '16 at 12:01
@Jens do you refer to my answer? The iterator is not incremented in case of erase. Probably, there were some old, deleted comments – IceFire Jul 18 '20 at 12:49
@IceFire I think the answer got edited, but I honestly don't remember. – Jens Jul 26 '20 at 12:08
@Jens jep, it’s been a long time. I wonder why I did not answer earlier. But well, whatever :) – IceFire Jul 26 '20 at 13:43

Jens · Answer 2 · 2016-03-05T12:12:27.283

2

Instead of rolling your own loop, you can (and should) use the standard library:

allObjects.erase( std::remove(myObjects.begin(), myObjects.end, myObjects[i]),
                 allObjects.end() );

This is more efficient because your algorithm is O(n^2) since the elements are shifted all the time, and it is more readable.

In your case, since you store pointers, you have to delete the "removed objects" first:

auto r = std::remove(myObjects.begin(), myObjects.end, myObjects[i]);
for(auto i=r; i != allObjects.end(); ++i) {
     delete *i;
}
allObjects.erase(r, allObjects.end());

This will be simpler if you use smart pointers (std:unique_ptr or std::shared_ptr) because you can skip the manual loop and just use the common one-liner.

edited Mar 05 '16 at 12:12

answered Mar 05 '16 at 12:04

Jens

9,058
2
26
43

My intentions were only to remove one element of the myObjects at index i from allObjects, so I think I would use something like std::remove(allObjects – doingit Mar 05 '16 at 13:27
@doingit I think the comment is cut off. If you want to remove an object, you need to call erase on the vector. If you know that there is at most one occurence, you can use std::find instead of std::remove. – Jens Mar 05 '16 at 13:32
Yeah it was, apologies, but yes. Does std::find work with pointers? – doingit Mar 05 '16 at 13:37
@doingit `std::find` works with everything implementing `operator==` which includes pointers. – Jens Mar 05 '16 at 19:01

score 0 · Answer 3 · answered Mar 05 '16 at 12:41

objects.erase(
    std::remove_if(
        objects.begin(),
        objects.end(),
        [](const Object &) -> bool {
            // Do "some stuff", then return true if element should be removed.
            return true;
        }
    ),
    objects.end()
);

You can also use Boost.Range and Lambda.

c++ How to erase object using vector with auto while inside loop

3 Answers3