C++ Array of Pointers: Delete or Delete []

C++ Array of pointers: delete or delete []?

delete[] monsters;

Is incorrect because monsters isn't a pointer to a dynamically allocated array, it is an array of pointers. As a class member it will be destroyed automatically when the class instance is destroyed.

Your other implementation is the correct one as the pointers in the array do point to dynamically allocated Monster objects.

Note that with your current memory allocation strategy you probably want to declare your own copy constructor and copy-assignment operator so that unintentional copying doesn't cause double deletes. (If you you want to prevent copying you could declare them as private and not actually implement them.)

Deleting array of pointers

No, delete [] is used to delete an array. If you need to delete array elements, you need to call delete on each one of them.

Proper way to delete an array of pointers

Every pointer allocated with new gets a corresponding delete. Every pointer allocated with new [] gets a corresponding delete []. That's really all you need to know. Of course, when you have a dynamically allocated array which contains dynamically allocated pointers the deallocation must occur in reverse order.

So it follows that the correct idiom would be...

int main()
{
    int **container = new int*[n];
    for(int i = 0; i < n; ++i)
        container[i] = new int[size];

    // ... and to deallocate...
    for(int i = 0; i < n; ++i)
        delete [] container[i];

    delete [] container;
}

And then of course I say "stop doing that" and recommend you use a std::array or std::vector (and the template type would be unique_ptr<int>).

C++: How to properly delete array of pointers to pointers in class destructor

In all of the similar questions I found, each element of the array is assigned a value dynamically, using new as such: arr[i] = new First(); . However, here the elements are assigned the value of a pointer to an object that is a parameter of the function. So, should the destructor delete every element one by one and then delete the array, or is it enough to delete the array?

That, we cannot answer. Does Second take ownership of the objects passed to .add(), and if so how were they allocated?

1. If it does not take ownership, just deleting the array is enough, and that array should be managed by a std::unique_ptr doing so for you.

2. If it does take ownership, that argument to .add() should be the smart-pointer with the right ownership-semantics and deleter. Your array should then be an array of those smart-pointers, managed by a std::unique_ptr.

In either case, if you properly use smart-pointers, the default-dtor is fine.

how to properly delete a pointer to array

The requirement to match new[] with delete[] is technically correct.

Much better, however (at least in my opinion), would be to forget that you ever even heard of new[], and never use it again. I'm pretty sure it's been (at least) 10 years since the last time I used new[], and if I'd understood the situation very well, I'd have stopped even sooner than that. Pretty nearly any time you'd even consider using new[], an std::vector (or possibly std::deque) will be a better choice.

If you're trying to create something roughly equivalent to a vector or deque yourself, you don't normally want to use new[] for that either. They way they (at least normally, though it's possible to change this via a custom Allocator class) is to allocate "raw" storage with operator new (which is pretty much like malloc--you just give it a size, and it gives you that many bytes of storage). Then you use the placement new operator to create objects in that space, and explicitly invoke the object's destructor to destroy objects in that space.

To give one example, this is what allows std::vector to support reserve, which allows you to allocate extra space, which won't be turned into objects until you call something like push_back or emplace_back to create an actual object in the space you allocated.

When you use new[], it has to create objects of the specified type filling all the space you allocate. You can't create something like push_back that adds a new object to the collection, because the collection is always already "full". All you can do is allocate a new collection that's larger than the old one (so every addition to the collection is O(N) instead of the amortized O(1) supported by std::vector--a huge loss of efficiency).

Removing elements from an array of pointers - C++

I thought that since each element of this array was a pointer to a structure, then in order to delete 42, I would have to first call delete on arr[2]

Yes, you would use delete on arr[2] in order to free the memory to which arr[2] is pointing.

then I would say arr[2] = arr[3]

So far so good.

and then delete on arr[3]

This is the problem right here. Suppose you have code like this:

int arr_cnt = 5;
int *arr[arr_cnt];
for(int i = 0; i < arr_cnt; ++i)
    arr[i] = new int(i+arr_cnt);  // using ints for simplicity, but concept is the same

Your array arr now looks like this:

idx   *arr[]     values in heap, due to using 'new' operator
    +-----+
0   | a---|----> 5
    +-----+
1   | b---|----> 6
    +-----+
2   | c---|----> 7
    +-----+
3   | d---|----> 8
    +-----+
4   | e---|----> 9
    +-----+

Where the letters represent different memory addresses returned by new.

This means that, to properly remove element at index 2, you need to:

1. use delete on arr[2] to avoid a memory leak,
2. overwrite arr[2] with some other address that's still valid (using arr[2] before this step would trigger a seg-fault)
3. nullify array position copied into arr[2] (see below)
4. decrement the length of your array (e.g. arr_cnt--;)

In other words:

delete arr[2];      // step 1, address 'c' no longer valid
arr[2] = arr[4];    // step 2, arr[2] is now 'e', which points to 9 just like arr[4]
arr[4] = NULL;      // step 3, arr[4] is now invalid
--arr_cnt;          // step 4

The diagram would now look like this:

idx   *arr[]     values in heap, due to using 'new' operator
    +-----+
0   | a---|----> 5
    +-----+
1   | b---|----> 6
    +-----+
2   | e---|-----------+   // address 'e' used to be in arr[4]
    +-----+           |
3   | d---|----> 8    |
    +-----+           |
4   | nil |      9 <--+
    +-----+

then arr[3] = arr[4]. That wasn't working

If you follow the diagram, you've probably noticed by now that using delete on both meant that you were invalidating two entries. In other words, if we skip the 2nd diagram and try your delete arr[2]; arr[2] = arr[3]; delete arr[3] logic, you end up with:

delete arr[2];
    +-----+
0   | a---|----> 5
    +-----+
1   | b---|----> 6
    +-----+
2   | c---|----> ?  // invalidated
    +-----+
3   | d---|----> 8
    +-----+
4   | e---|----> 9
    +-----+

arr[2] = arr[3];
    +-----+
0   | a---|----> 5
    +-----+
1   | b---|----> 6
    +-----+
2   | d---|-+
    +-----+ |
3   | d---|-+--> 8  // both have the same address, so point to the same place
    +-----+
4   | e---|----> 9
    +-----+

delete arr[3];
    +-----+
0   | a---|----> 5
    +-----+
1   | b---|----> 6
    +-----+
2   | d---|-+
    +-----+ |
3   | d---|-+--> ?  // both invalid now, but not set to null
    +-----+
4   | e---|----> 9
    +-----+

so I just decided to try it without the delete keyword and just do arr[2] = arr[3] and arr[3] = arr[4], and it worked.

But now you have a memory leak. You always have to delete every new in C++, just like you always have to free every malloc in C.

So my question is, why did I not have to use the delete keyword in order to do this.

You do have to use it. The problem is that you were invalidating more than you thought you were and ended up trying to work with memory that had been deallocated. When a program tries to access memory like this, it causes a segmentation fault. I don't remember the exact message Windows would display, but it's probably an unhandled exception message of some kind. (Segmentation fault tends to be GNU/Linux terminology.)

I was thinking that if I just set arr[2] to arr[3], then the structure being pointed to by arr[2] would be lost, and I would get a memory leak. Is that not the case?

You're correct here. The problem is not your understanding of the new/delete relationship, only the fact that the assignment you described is a shallow copy and you ended up deleting more than expected.

Remove an object from an array of pointers

Your operator- is not implemented correctly. Although its loop to find an object is fine, removing that object is broken. You are not updating the array correctly, as you need to shift down ALL of the array elements after the found index, which you are not doing.

Also, make sure your List class implements the Rule of 3/5/0 correctly. The return value of operator- is supposed to return a new List object, so a copy has to be made. You should not be modifying the this object at all in operator- (that is the job of operator-= to do; see What are the basic rules and idioms for operator overloading?).

Also, the buildingNr member should not be static. That prevents you from using multiple List objects at a time.

Try something more like this instead:

#include <algorithm>

class List {
    Building* buildingList[10];
    int buildingNr;
    
public:
    
    List() : buildingNr(0) {}

    List(const List &src) : buildingNr(src.buildingNr) {
        for(int i = 0; i < buildingNr; ++i) {
            buildingList[i] = new Building;
            buildingList[i]->id = src.buildingList[i]->id;
            buildingList[i]->year = src.buildingList[i]->year;
            buildingList[i]->price = src.buildingList[i]->price;
            buildingList[i]->adress = new char[strlen(src.buildingList[i]->adress)+1);
            strcpy(buildingList[i]->adress, src.buildingList[i]->adress);
        }
    }

    ~List() {
        for(int i = 0; i < buildingNr; ++i) {
            delete[] buildingList[i]->adress;
            delete buildingList[i];
        }
    }

    List& operator=(const List &rhs) {
        if (this != &rhs) {
            List tmp(rhs);
            std::swap_ranges(buildingList, buildingList+10, tmp.buildingList);
            std::swap(buildingNr, tmp.buildingNr);
        }
        return *this;
    }

    List& operator-=(int id) {    
        for(int i = 0; i < buildingNr; ++i) {
            if (buildingList[i]->getId() == id) {
                delete buildingList[i];
                for(int j = i + 1; j < buildingNr; ++j) {
                    buildingList[j - 1] = buildingList[j];
                }
                buildingList[buildingNr] = NULL;
                --buildingNr;
                cout << "Building " << i << " was deleted." << endl;
                return *this;
            }
        }
        throw -1;
    } 

    ...
};

friend List operator-(List lhs, int id) {    
    lhs -= id;
    return lhs;
} 

---

Related Topics