9

I have a class that creates a vector of objects. In the deconstructor for this class I'm trying to deallocate the memory assigned to the objects. I'm trying to do this by just looping through the vector. So, if the vector is called maps I'm doing:

Building::~Building() {
    int i;
    for (i=0; i<maps.size(); i++) {
        delete[] &maps[i];
    }
}

When I run this the program segfaults while deallocating memory. I think what I'm doing is actually deleting the array storing the objects and not the objects themselves. Is this correct? If not any ideas as to what I'm doing wrong?

Unihedron
  • 10,902
  • 13
  • 62
  • 72
Ian Burris
  • 6,325
  • 21
  • 59
  • 80
  • 9
    What is the type of the `vector`? From what it looks like, your `vector` contains _objects_, not _pointers to objects_. If that is the case, then you don't delete them; the `vector` takes care of all the cleanup. [Of course it's hard to say because this code isn't exactly the code you are using, because `size` is a member function, not a member variable; this code wouldn't work at all.] – James McNellis Sep 27 '10 at 22:51
  • How does your vector declaration look like, and, more important: Why do you want to do this? – Jim Brissom Sep 27 '10 at 22:52
  • do you mean std::vector or do you mean 'vector' as alternate name for array? – pm100 Sep 27 '10 at 22:57
  • Although the syntax is wrong (missing parantheses), he is using `size`, so I'd guess he is really talking about `std::vector`. – Jim Brissom Sep 27 '10 at 23:00
  • Yes, I'm talking about std::vector. And you guys are right, I need to add the parentheses. – Ian Burris Sep 27 '10 at 23:13
  • 1
    show us the vector declration – pm100 Sep 27 '10 at 23:18
  • 2
    Since `.size()` return type is `size_t`, prefer using `size_t` as the type of index variable (here `i`). Unless of course you are using C++0x, where you can use `auto`. Also prefer limiting the scope of 'i` to as local as possible, e.g. only inside `for` loop. Finally choose a different variable name, because those who don't see the declaration, may incorrectly assume that `maps` is a plurality of `std::map`. – Arun Sep 27 '10 at 23:51
  • I would like to emphasize the use of iterators here while looping over the container that has been named maps in the question, in that case maps can point to any container but destructor code remains the same. – Manish Chowdhary Nov 26 '15 at 16:21
  • @blcArmadillo - Adding to what Arun mentioned, I would also "insert" the iterator variable `i` into the for statement, i.e.- for (size_t i = 0; i < maps.size(); ++i) – Guy Avraham Apr 07 '18 at 09:32
  • Whether of not you should call "delete" on ANY object depends on whether the object exists on heap or the stack. YOU get to decide where the memory for that object is allocated. When you do something like MyClass *myclass = new MyClass(...), you have created the pointer to the class whose memory was allocated on the heap. The destructors for the objects on the heap will not be called when it "goes out of scope". For the objects on the stack, destructors will be automatically called. So it all depends on how the objects that you're pushing on the vector were created. – sdevikar Apr 02 '19 at 20:43

7 Answers7

21

It depends on how vector is defined.

If maps is a vector<myClass*> you delete each element with something similar to:

for ( i = 0; i < maps.size(); i++)
{
    delete maps[i];
}

If maps is a vector<myClass> I don't think you need to delete the individual elements.

Alex Reece
  • 1,906
  • 22
  • 31
  • 2
    This is only half of the story. You can allocate the pointer as you are saying yet still do it wrong. This is the thrust of what I was getting at in my post. Certainly you are correct, but understanding the types is the easy part (he's already figured half of it out) because the compiler tells you. It seems to me that he has a slight misunderstanding of memory management fundamentals. – San Jacinto Sep 28 '10 at 00:40
  • i have tried this but after delete vec[i] , vector size still same and object is there. – Muhammad Shauket Nov 24 '17 at 07:07
18

It's hard to tell from the terminology you've used and the code presented exactly what's going on. So maybe a few examples would help you out.

Array new and array delete

What's up with new [] and delete [] you ask? These guys are used for allocating/deallocating arrays of things. Those things could be POD or they could be full fledged objects. For objects they will call the constructor after allocating and destructor while deallocating.

Let's take a contrived example:

class MrObject
{
public:
   MrObject() : myName(new char[9]) { memcpy(myName, "MrObject", 9); }
   virtual ~MrObject() { std::cout << "Goodbye cruel world!\n"; delete [] myName; }
private:
   char* myName;
};

Now we can do some fun stuff with MrObject.

Arrays of objects

First let's create a nice and simple array:

MrObject* an_array = new MrObject[5];

This gives us an array of 5 MrObjects, all nicely initialized. If we want to delete that array we should perform an array delete, which in turn will call the destructor for each MrObject. Let's try that:

delete [] an_array;

But what if we goofed up and just did a normal delete? Well now's a good time to try it for yourself

delete an_array;

You'll see that only the first destructor get's called. That's because we didn't delete the whole array, just the first entry.

Well sometimes. It's really undefined what happens here. The takeaway is to use the array form of delete when you use an array new, ditto for just plain old new and delete.

Vectors of Objects

OK, so that was fun. But let's take a look at the std::vector now. You'll find that this guy will manage the memory for you, and when he goes out of scope, well so does everything he's holding onto. Let's take him out for a test ride:

std::vector<MrObject> a_vector(5);

Now you have a vector with 5 initialized MrObjects. Let's see what happens when we clear that sucker out:

a_vector.clear();

You'll note that all 5 destructors got hit.

Vectors of Pointers to Objects

Oooooh you say, but now lets get fancy. I want all the goodness of the std::vector, but also want to manage all the memory myself! Well there's a line for that as well:

std::vector<MrObject*> a_vector_of_pointers(5);
for (size_t idx = 0; idx < 5; idx++) {
   // note: it's just a regular new here, not an arra
   a_vector_of_pointers[idx] = new MrObject;
}

See that was a bit more of a pain. But it can be useful, you could use a non-default constructor when creating MrObject. You could put derived MrObjects in there instead. Well as you can see the sky's the limit. But wait! You created that memory, you best manage it. You'll want to loop over each entry in the vector and cleanup after yourself:

for (size_t idx = 0; idx < a_vector_of_pointers.size(); idx++) {
   delete a_vector_of_pointers[idx];
}
Eric Rahm
  • 688
  • 3
  • 5
  • @Eric Rahm +1 for the good and comprehensive answer - yet I think it is worth mention here that all that has to do with deleting the array of objects wont work in the case that the array is of some derived class, and the pointer to the array is of base type. See the answer here: https://stackoverflow.com/a/6171991/1971003 – Guy Avraham Oct 15 '17 at 18:07
6

In C++, you can only delete data by pointer. You've accomplished this using the & operator, but if your vector doesn't contain pointers that point to memory allocated on the machines heap (not the stack, as is the method when you have a normal variable declaration) then you can TRY to delete it, but you will encounter undefined behavior (which will hopefully cause a program crash).

When you insert into a vector, the vector calls the class's copy constructor and you're actually inserting a copy of the object. If you have a function whose sole purpose is like the following:

void insertObj(obj & myObject)
{
  myVector.insert(myObject);
}

Then realize that there are two obj's in this scope: the one you passed in by reference, and the copy in the vector. If instead we had pass in myObject by value and not by reference, then we could say that two copies of the object exist in this scope, and one exists in the caller. In each of these 3 instances, they are not the same object.

If you are instead storing pointers in the container, then the vector will create a copy of the pointer (NOT a copy of the object) and will insert the copied pointer into the vector. It is usually not a good practice to insert elements into a container by pointer unless you know that the object will live at least until the container is done with it. For example,

void insert()
{
  Obj myObj;
  myVector.insert(&myObj);
}

Is probably a very bad idea, as you'd have a pointer in the vector that points to an object that is destroyed automatically when it goes out of scope!

Point being, if you malloc'd or new'd your object, then you need to free or delete it. If you created it on the stack, then do nothing. The vector will take care of it when it is destroyed.

For a deeper understanding of stack-based allocation vs. heap-based allocation, see my answer here: How does automatic memory allocation actually work in C++?

Community
  • 1
  • 1
San Jacinto
  • 8,774
  • 5
  • 43
  • 58
1
for(std::vector<MyObjectClass>::iterator beg = myVector->begin(), end = myVector->end(); beg != end; beg++)
{
    delete *beg;
}
myVector->clear();
Roman Solodyashkin
  • 799
  • 12
  • 17
1

I decided to turn my comment into an answer (along with the other great answers here), so here it goes.

I would note again, that this case deals with inheritance of the object.

When you delete an array of Derived object, pointed by a Base pointer, as follows:

Base* pArr = new Derived[3];

delete [] pArr;

What the compiler does "under the hood" is to generate the following code:

//destruct the objects in *pArr in the inverse order
//in which they were constructed
for (int i = the number of elements in the array - 1; i >= 0; --i)
{
     pArr[i].Base::~Base(); 
}

Now, when doing so, we get undefined behavior. Dealing with arrays is simply dealing with offsets so when this loop occurs, in each iteration of the loop the pointer of the array is incremented according to the size of Base --> and here is where things becomes "undefined". In the "simple" (yet less common) case where the Derived class does not add any members of its own, its size is as the size of Base --> so things might (I guess that not always) work well. But (!!) when you add at least one member to the Derived class, its size grows, causing the offset increment in each iteration to be wrong.

To illustrate this case I have create the following Base and Derived objects. Note that in the case that Derived does not contain the m_c member, the delete operation goes well (comment it out and see for yourself), YET once you add it, I got a segmentation fault (which is the undefined behavior).

#include <iostream>
using namespace std;

class Base 
{

    public:
        Base(int a, int b)
        : m_a(a)
        , m_b(b)    
        {
           cout << "Base::Base - setting m_a:" << m_a << " m_b:" << m_b << endl;
        }

        virtual ~Base()
        {
            cout << "Base::~Base" << endl;
        }

        protected:
            int m_a;
            int m_b;
};


class Derived : public Base
{
    public:
    Derived() 
    : Base(1, 2) , m_c(3)   
    {

    }

    virtual ~Derived()
    {
        cout << "Derived::Derived" << endl;
    }

    private:    
    int m_c;
};

int main(int argc, char** argv)
{
    // create an array of Derived object and point them with a Base pointer
    Base* pArr = new Derived [3];

    // now go ahead and delete the array using the "usual" delete notation for an array
    delete [] pArr;

    return 0;
}
Guy Avraham
  • 3,482
  • 3
  • 38
  • 50
0

It's hard to say from your question just what the signature of maps is. I'm guessing you want to use delete[] because you also used new[]. So does that mean the members of your vector is itself a collection? supposing it is, then you have something like this:

class Building {
  public:
    typedef int* maps_t;
  private:
    std::vector<maps_t> maps;
  public:
    Building();
    ~Building();
};

Building::Building(size_t num_maps) {
  for(;num_maps; --num_maps)
  {
    maps.push_back(new Building::maps_t[10]);  
  }
}

in that case, your destructor is nearly right; you need change only &maps[i] to maps[i].

Building::~Building() {
    int i;
    for (i=0; i<maps.size(); i++) {
        delete[] maps[i];
    }
}

But in C++, we rarely like to do things that way. For one thing, unless you are actually trying to implement something like std::vector, you rarely want to use new[] or delete[] explicitly. You can, for example, use std::vector. You need perform no explicit memory management in that case. Your class will look like so:

class Building {
  public:
    typedef std::vector<int> maps_t;
  private:
    std::vector<maps_t> maps;
  public:
    Building();
};

Building::Building(size_t num_maps) {
  for(;num_maps; --num_maps)
  {
    maps.push_back(Building::maps_t(10));  
  }
}

There is no user defined destructor in this case, because std::vector already manages its own memory quite well.

SingleNegationElimination
  • 151,563
  • 33
  • 264
  • 304
  • May want to mention that the "rarely" you refer to would be eg. where the stack simply isn't big enough for the data you need to hold. – Engineer Oct 19 '11 at 14:56
-2

If you're using std::vector then you can just let it get handled by the destructor for vector, assuming there are "objects" (and not pointers to objects) in said vector.

-- or --

If you're using a standard array as the "vector":

The purpose of the "delete []" variation is to deallocate an entire array, and hence avoid the need to have a for loop like you do.

If using standard C/C++ arrays, "delete [] maps" should do it for you. "[]" shouldn't be used for deallocating STL vectors.

Unihedron
  • 10,902
  • 13
  • 62
  • 72
John Carter
  • 6,752
  • 2
  • 32
  • 53
  • -1 because he is not really talking about arrays here. Deleting a std::vector that way is really a bad idea, apart from not really being applicable. After the edit, still -1 for suggesting to manually call the destructor - which is also a pretty bad idea, seeing as it will most likely get called again when the object goes out of scope. The vector can be emptied via `swap` if needed, though, but even that is seldom necessary. – Jim Brissom Sep 27 '10 at 23:05
  • You're right, I didn't mean to suggest calling the destructor explicitly. – John Carter Sep 27 '10 at 23:10