How to Properly Clean Up Elements from Vectors of Object Pointers

How to properly clean up elements from vectors of object pointers

you mentioned in the comments that the objects will have other pointers to them. this sounds like std::shared_ptr<A> to me. this way you can have other pointers to your A objects without memory leaking issues. (std::shared_ptr comes with a small(!) performance cost, but you should not worry about it for now).
additionally i changed your passage to delete/erase your element from the vector. be aware that the A objects are still alive IF there are other instances keeping a std::shared_ptr<A> copy to it (but thats a good thing).

here is the code:

#include <iostream>
#include <vector>
#include <memory>
#include <algorithm>

class A
{
public:
    A(int i) { x = i; }
    int x;
};

int main()
{
    std::vector<std::shared_ptr<A>> Vec;
    Vec.emplace_back(std::make_shared<A>(5));
    Vec.emplace_back(std::make_shared<A>(4));
    Vec.emplace_back(std::make_shared<A>(3));

    std::cout << "Size before = " << Vec.size() << std::endl;

    Vec.erase(
        std::remove_if(std::begin(Vec),std::end(Vec), [](auto&& ptr){ return ptr->x == 3;}),
        std::end(Vec));

    std::cout << "Size after = " << Vec.size() << std::endl;
    for (auto&& a : Vec)
    {
        std::cout << a->x << std::endl;
    }

    return 0;
}

clearing a vector of pointers [duplicate]

Yes, the code has a memory leak unless you delete the pointers. If the foo class owns the pointers, it is its responsibility to delete them. You should do this before clearing the vector, otherwise you lose the handle to the memory you need to de-allocate.

   for (auto p : v)
   {
     delete p;
   } 
   v.clear();

You could avoid the memory management issue altogether by using a std::vector of a suitable smart pointer.

Does vector::erase() on a vector of object pointers destroy the object itself?

vector::erase

Removes from the vector container and calls its destructor but If the contained object is a pointer it doesnt take ownership of destroying it.

You will have to explicitly call delete on each contained pointer to delete the content it is pointing to, for example:

void clearVectorContents( std::vector <YourClass*> & a ) 
{    
    for ( int i = 0; i < a.size(); i++ ) 
    {       
        delete a[i];    
    }    
    a.clear(); 
} 

Storing raw pointers in standard containers is not a good idea. If you really need to store resources that have to be allocated by new, then you should use boost::shared_ptr. Check out the Boost documentation.

An more generic & elegant solution:

This solution makes use of for_each & templates as @Billy pointed out in comments:

// Functor for deleting pointers in vector.
template<class T> class DeleteVector
{
    public:
    // Overloaded () operator.
    // This will be called by for_each() function.
    bool operator()(T x) const
    {
        // Delete pointer.
        delete x;
        return true;
    }
};

And this can be called as:

for_each( myclassVector.begin(),myclassVector.end(),
          DeleteVector<myclass*>());

where, myclassVector is your vector containing pointers to myclass class objects.

Usage Example:

#include "functional"
#include "vector"
#include "algorithm"
#include "iostream"

//Your class
class myclass
{
    public:
        int i;
        myclass():i(10){}
};


// Functor for deleting pointers in vector.
template<class T> class DeleteVector
{
    public:
    // Overloaded () operator.
    // This will be called by for_each() function.
    bool operator()(T x) const
    {
        // Delete pointer.
        delete x;
        return true;
    }
};


int main()
{
    // Add 10 objects to the vector.
    std::vector<myclass*> myclassVector;

    for( int Index = 0; Index < 10; ++Index )
    {
        myclassVector.push_back( new myclass);
    }

    for (int i=0; i<myclassVector.size(); i++) 
    {
        std::cout << " " << (myclassVector[i])->i;
    }

    // Now delete the vector contents in a single  line.
    for_each( myclassVector.begin(),
              myclassVector.end(),
              DeleteVector<myclass*>());

    //Clear the vector 
    myclassVector.clear();

    std::cout<<"\n"<<myclassVector.size();

    return 0;
}

How to clean up class member of pointer to vector of pointers to a class?

The number of pointers is a bit ridiculous, as all they are doing is causing confusion and leaks, as evident from non-proper initialization and the question's title. You don't actually need any pointers at all, and don't have to worry about any cleanup.

For a 2D array with the first dimension passed into the constructor, you can use a vector of vectors:

std::vector<std::vector<Bar>> bars; 

To initialize the outer vector with the passed in size, use an initializer:

Foo(size_t size) 
    : bars(size) {}

When the object is destroyed, bars and all of it elements are as well, so there's no chance of forgetting to clean up or doing so improperly.

If performance is an issue, this can be translated into a sort of Matrix2D class that acts like a 2D array, but really only has an underlying 1D array.

Cleaning up an STL list/vector of pointers

Since we are throwing down the gauntlet here... "Shortest chunk of C++"

static bool deleteAll( Foo * theElement ) { delete theElement; return true; }

foo_list . remove_if ( deleteAll );

I think we can trust the folks who came up with STL to have efficient algorithms. Why reinvent the wheel?

How do vectors clean up after themselves? (Or do they?)

When you use concrete objects (e.g. Bullet) instead of pointers to objects you don't need to use delete.

So bullets[i] = myBullet makes a copy of myBullet. It assigns the old value of bullets[i] with the new value of myBullet. When the vector goes out of scope it will call each of it's elements destructor which cleans up any memory held by the element.

To be technically correct, the above statement uses the assignment operator (operator=), to make the copy. That does whatever is needed to clean up the old bullet and copy the new bullet's member variables to the old bullet's member variables.

bullets.push_back(myBullet) is similar. Except it adds myBullet to the end of the vector. So that means if the vector had a size of 9 it will now be 10.

To be technically correct, the push_back uses a copy constructor, as opposed to the assignment operator, to construct the new object (I think) since there is no existing old bullet object.

The bottom line is that given a correctly implemented concrete class (no matter how complicated it is internally) you can use instances of them in std::vector and not worry about doing any memory cleanup.

Deleting objects from a vector of pointers

As a number of comments have pointed out, vector.erase only removes the elements from the vector. It does NOT try to delete any associated memory.

To delete the associated memory explicitly, you need to:

int main(int argc, const char * argv[])
{
    ...

    auto it = objs.begin() + i;
    delete *it;
    objs.erase(it);

}

Actually, in your case:

int main(int argc, const char * argv[])
{
    std::vector<Obj*> objs;

    Obj* o = new Obj(10);
    objs.push_back(o);    

    auto it = objs.begin();
    delete *it;
    objs.erase(it);

}

There are a number of other inconsistencies with your code and, better solutions for what you're trying to do, such as:

• Using a vector<Obj>:

  int main(int argc, const char * argv[])
  {
      std::vector<Obj> objs;
      objs.emplace_back(10);    
  
      auto it = objs.begin();
      objs.erase(it);
  }
  

• If you need to dynamically allocate your objects, but for some reason do not want the vector to handle that, you can use shared_ptr or unique_ptr, who will take care of the deallocation for you:

  int main(int argc, const char * argv[])
  {
      std::vector<std::unique_ptr<Obj>> objs;
  
      objs.emplace_back(new Obj(10));    
  
      auto it = objs.begin();
      objs.erase(it);
  }
  

How to clean up a vector/map properly?

No, you must iterate through the vector/ map, remove and delete its items one by one (which, as @SB pointed out, may require disposing of their members recursively).

(You could get away by simply deleting the items, if you are absolutely sure no one will access the vector elements anymore before the vector gets deleted - but it is still safer to remove each item before deleting it. This ensures program correctness at any point, eliminating the possibility for subtle bugs and easing maintenance in the long term.)

By the way this is one of the reasons why it is recommended to store smart pointers in collections, instead of raw pointers.

How to erase & delete pointers to objects stored in a vector?

You need to be careful because erase() will invalidate existing iterators. However, it will return a new valid iterator you can use:

for ( it = Entities.begin(); it != Entities.end(); ) {
   if( (*it)->getXPos() > 1.5f ) {
      delete * it;  
      it = Entities.erase(it);
   }
   else {
      ++it;
   }
}

C++ how do you delete 2 pointers located in two vectors at the same time?

I want the pointer to be removed in each vector. Is this possible in C++ without iterating through the vector.

No, it is not. However, you do not have to iterate manually, you can use STL algorithms instead, eg:

A.erase(std::remove(A.begin(), A.end(), ptr), A.end());
B.erase(std::remove(B.begin(), B.end(), ptr), B.end());

Or:

auto iter = std::find(A.begin(), A.end(), ptr);
if (iter != A.end()) A.erase(iter);

auto iter = std::find(B.begin(), B.end(), ptr);
if (iter != B.end()) B.erase(iter);

I'm assuming I can also do a check to see if A[i] == NULL?

You assume incorrectly, since you are not storing any NULL pointers in the vector.

---

Related Topics