I have a vector of pointers and I want to delete an entry. After I delete the entry, I check the size of the vector and it is not reduced. Should I do an extra thing?
printf("size of vector before %d \n", m_obj.size());
for(std::vector<object*>::iterator it = m_obj.begin(); it != m_obj.end(); ++it)
{
if((*it)->m_id == p_id)
{
delete *it;
}
}
printf("size of vector after %d \n", m_obj.size());
Your call to delete *it merely deletes the object pointed to by that entry in your vector. You do not modify the vector itself.
To also remove the element from the vector, make use of the erase method:
delete *it;
it = m_obj.erase(it);
Note that erasing invalidates the current iterator, so you will probably want to assign the return value of the erase call to your it variable. Be careful not to accidentally skip elements when erasing more than one entry in a loop though. The returned iterator will point to the element after the one that was deleted and will get incremented again by the for loop.
The easiest way to get rid of this issue is to use an STL algorithm for removing the elements instead. This works best if the entries also have ownership of their pointees, so you might want to consider using vector of smart pointers instead (as suggested in the comments), so you don't have to worry about explicitly calling delete at all.
+----------+ +-------------+
| object * |-------->| heap memory |
+----------+ +-------------+
it-------->| object * |-------->| heap memory |
+----------+ +-------------+
| object * |-------->| heap memory |
+----------+ +-------------+
| object * |-------->| heap memory |
+----------+ +-------------+
After delete *it:
+----------+ +-------------+
| object * |-------->| heap memory |
+----------+ +-------------+
it-------->| object * |-------->
+----------+ +-------------+
| object * |-------->| heap memory |
+----------+ +-------------+
| object * |-------->| heap memory |
+----------+ +-------------+
You also need to somehow erase the pointer element in your vector. Keep in mind that you can't do erase(it) during the iteration since it invalidates your iterator.
You are only freeing memory allocated under the pointer. You're not removing the pointer itself.
After you call delete, set the value of pointer to NULL. After your for loop use std::remove_if with NULL check on elements.
Deleting the object will not remove the pointer to it from the vector, it will only free the memory pointed to by the pointer. You will need to explicitly call erase on the vector after the element is freed.
Having an STL container (e.g. std::vector) of owning raw pointers is in general a bad idea. This can be a source of leaktrocity: e.g. if an exception is thrown somewhere, the vector destructor is called, but that will not delete the raw owning pointers! So their associated resources are leaked.
Moreover you have to provide explicit cleanup code, etc.
You may want to either use std::vector<Object>, or a std::vector of smart pointers, e.g. vector<shared_ptr<Object>> or vector<unique_ptr<Object>> (the latter available since C++11 thanks to move semantics, and a good solution if the vector is the unique owner of the objects, and there is no shared ownership).
You may also want to use algorithms like remove_if(). In particular, to remove items satisfying some precise condition from vector, you may want to use the so called erase-remove idiom:
// Use a vector of *smart pointers* (shared_ptr or unique_ptr).
typedef std::shared_ptr<Object> ObjectPtr;
std::vector<ObjectPtr> v;
...
...
// Use the erase-remove idiom to erase items satisfying a particular condition.
v.erase( std::remove_if(v.begin(), v.end(), [&](const ObjectPtr & p) {
return p->m_id == deleteId; // Erasing condition
}), v.end());
The make the code clearer (especially if one doesn't know/recognize the erase-remove idiom), it's possible to write a little helper function to implement this idiom:
// Helper function to implement the erase-remove idiom for std::vector.
template <typename T, typename Predicate>
void erase_if(std::vector<T>& v, Predicate p) {
v.erase(std::remove_if(v.begin(), v.end(), p), v.end());
}
And then call it like this:
// Simplified code: just pass the vector 'v' and the erasing condition
// (in this case using a simple lambda).
// Uses the erase-remove idiom under the hood.
//
erase_if(v, [&](const ObjectPtr & p) { return p->m_id == deleteId; });
Note that thanks to the use of the remove_if algorithm and the erase-remove idiom, the semantic level of your code is raised: you don't read explicit for loops and iterator incrementing anymore; you can just read the clear intent of the code, i.e. erasing items that satisfy a given condition.
PS: You might also want to read: "How do I erase elements from STL containers?".
