using the function erase in std::vector

Question

I Have this function, its purpose is to delete a pointer of class BaseFile from a vector called children

 //children is vector of type vector<BaseFile*>
void Directory::removeFile(BaseFile* file)
{
   for(int i = 0 ; (unsigned)i < children.size() ; i++)
    {
        if ((children[i]->getName()).compare(file->getName()) == 0)
        {    
            BaseFile* a = children[i];
            children.erase(children.begin()+i);
            if(a != nullptr) 
            {
                delete a;//err in this line : double free or corruption
            }
        } 
    }
}

first question is why I AM getting an error in the line (delete a;) ? dose the method erase removes the pointer an delete it? if yes how can i remove the pointer from the vector without deleting it's content in Heap/Stack?

Please refer to the "Member functions" section of the [std::vector page on cppreference.com](http://en.cppreference.com/w/cpp/container/vector). Let us know if you need something else. — Ivan Aksamentov - Drop, Nov 14 '17 at 06:45
this code is flawed if you want to erase more than one item in one iteration. for the rest just read the docs indeed — Serve Laurijssen, Nov 14 '17 at 06:50
I already did... the documentation didnt help that much ; all the examples in the documentation are for integers iam asking in acase where i have a pointers stored in vecor, when erasing them using erase do i need to delete ? if yes then why iam getting error in the line (delete a;) if no how can i remove the pointer from the vector without deleting it,, please try to help . — Baraa Natour, Nov 14 '17 at 07:06
I think the error is not in the code shown, but it is flawed as Laurijssen already said. Did you add the same `BaseFile` twice to the vector, for example? — alain, Nov 14 '17 at 07:10
nope for now i wrote some functions and im testing them ,the BaseFile* im deleting is the only one — Baraa Natour, Nov 14 '17 at 07:20
at end i understood that i was trying to delete an object from the stack , it a little bit stupid but still posting my question here helped me a lot:) — Baraa Natour, Nov 14 '17 at 09:14

Rerito · Accepted Answer · 2017-11-14T10:06:27.563

What you need to do is to use std::remove_if to obtain the vector without the matching element.

However, once you have performed the call to std::remove_if you have no way to delete the matching items as the documentation states (emphasis mine):

Removing is done by shifting (by means of move assignment) the elements in the range in such a way that the elements that are not to be removed appear in the beginning of the range. Relative order of the elements that remain is preserved and the physical size of the container is unchanged. Iterators pointing to an element between the new logical end and the physical end of the range are still dereferenceable, but the elements themselves have unspecified values (as per MoveAssignable post-condition).

Therefore we'll handle the deletion directly in the predicate. Note that we must also take care not to double free anything so we'll keep track of the deleted item through the use of an std::unordered_set

void Directory::removeFile(BaseFile *file) {
     std::unordered_set<BaseFile*> deleted_set { file }; // Just to avoid deleting the input argument if it's stored in children as well...
     auto match_it = std::remove_if(begin(children), end(children),
          [&](BaseFile *current_file) -> bool {
              bool result = current_file->getName().compare(file->getName()) == 0;
              if (result && end(deleted_set) == deleted_set.find(current_file)) {
                  delete current_file;
                  deleted_set.insert(current_file);
              }
              return result;
          });
     children.erase(match_it, end(children));
}

Finally, I hope that the pointer you give as the file argument isn't a member of children as well and if it is, that you don't end up delete-ing it!

Note: Wouldn't it be possible to use smart pointers in your case? It seems that the Directory object has ownership on the BaseFile objects stored in children... So maybe std::unique_ptr would help...

i have some questions left in the for loop why should i use[for(....; ++it)] and not [for(....; it++)] and the second question is :- in your code(which i thank u for it) (*it) is a pointer to the end of my new vector ,its an element that i don't want to delete shouldn't it be (it = match_it +1) — Baraa Natour, Nov 14 '17 at 09:07
@BaraaNatour `++it` or `it++` doesn't really matter though you can notice a performance improvement using the former (see https://stackoverflow.com/a/38948073/1794345) — Rerito, Nov 14 '17 at 09:53
@BaraaNatour I just noticed that you can't delete the element after using `remove_if` as per its documentation. Give me some time to correct my answer and shed some light on your concerns — Rerito, Nov 14 '17 at 09:57

using the function erase in std::vector

1 Answers1