(memory leak?) Using new/delete with vectors in a destructor.

[lostwithcpp]

Hello,

I'm worried this code may cause a memory leak, its fairly straightforward otherwise. I wanted some feedback on how it would be best to delete the pointers in the destructor. I also wondered if I would need to use delete since I'm creating objects of type object within a class (named objects) and it gets destroyed on exit... am I wrong?

#include <iostream>
#include <vector>

using namespace std;

class object
{
    private:

    public:

    object(){}
    ~object(){}
};
class objects
{
   private:

        vector <object*> vector_of_objects;

   public:

        objects(){}
        ~objects()
        {
            cout<<"Destructor "<<endl;
            for (int c = 0; c < vector_of_objects.size(); c++)
            {
                object* temporary = vector_of_objects[c];
                delete temporary;
            }

            vector_of_objects.clear();
        }
        objects(int size)
        {
            for (int c = 0; c < size; c++)
                vector_of_objects.push_back(new object);
        }
};
int main()
{
    objects object(3);

    return 0;
}

[rapidcoder]

The problem with this code is when your "objects" object is copied.
Then the contents of the vector will be copied, and you will have 2 vectors with the exactly same pointer set. The destruction of any of those objects would leave you with dangling pointers in the other "objects" instance. Destroying the second object would yield a double-delete error.

You have to provide your own copy constructor to fix this, or instead of pointers use smart pointers from boost (but they are slooow).

[lostwithcpp]

The problem with this code is when your "objects" object is copied.

Hi,

Are you referring to this line (29)?

object* temporary = vector_of_objects[c];

...or are you giving me an example?

I think I would need to use a copy constructor for that line, just not sure how since a vector is involved.
I don't plan on using boost although I've heard of smart pointers.

EDIT:Is there any other way I would be able to delete the pointers in the vector without using a copy constructor ?

[rapidcoder]

I was giving you an example. That line is correct, as long as there is only a single vactor holding the same pointers. If there are two or more storing the same pointers (which would happen when the "objects" instance is copied by the default copy-constructor provided by the compiler), you are in big trouble.

[lostwithcpp]

I was giving you an example. That line is correct, as long as there is only a single vactor holding the same pointers. If there are two or more storing the same pointers (which would happen when the "objects" instance is copied by the default copy-constructor provided by the compiler), you are in big trouble.

Can you give me an idea of how I would create a copy constructor for the example you explained? thanks

Do you know if destroying the class leaves dangling pointers since the object is destroyed on exit?

[Silvermaul]

1)Copy constructor

object (const object &)
{
//copy logic here so that you don't have dangling pointers left
//copy the objects that the pointers point to
}

2) In the destructor call of the class don't forget to delete the "objects" that the pointers point to. If you don't the vector will be destroyed and then you will have no means of deleting those object i.e. memory leak.

To see if you did everything correctly create and delete an object dynamically, place everything in a loop and check your memory consumpiton. If it doesn't go up over time you are OK.