1

I'm having a little difficulty understanding the correct sequence of events that should happen when using the delete operator in C++. I've internalized that the proper way to use this is when a pointer is still referencing a pointee.

In the example below - I copy the contents of the array into temp and then delete [] the old array my arrayPointer was pointing too.

I then point the arrayPointer to the newly created array and set the no longer needed temp to nullptr. I want to make sure I'm not causing memory leaks by not deleting the temp pointer. Does this still need to happen?

I ask because I've seen examples where we point to nullptr first then delete but that seems counterintuitive. Any guidance would be greatly appreciated. Thanks!

    template <class T>
    void ValSet<T>::add(T elementToAdd){
        if(!this->contains(elementToAdd)){

            if(sizeOfArray == numOfElements){
                sizeOfArray *= 2;
                T* temp = new T[sizeOfArray];
                for (int i = 0; i < numOfElements; i++)
                    temp[i] = arrayPointer[i];
                delete [] arrayPointer;
                arrayPointer = temp;
                temp = nullptr;
            }
        numOfElements += 1;
        arrayPointer[numOfElements-1] = elementToAdd;
        }
      }
  • 1
    Your code looks OK to me. `temp = nullptr;` is unnecessary but harmless; `temp` is going out of scope soon anyway. – Igor Tandetnik May 23 '19 at 02:10
  • Side note, I think you want ` delete arrayPointer` not `delete [] arrayPointer`. –  May 23 '19 at 02:11
  • @Chipster - that's really interesting, but kinda makes sense. The member var is a pointer to T but when a ValSet object is constructed the arrayPointer makes a new array -- so does delete apply to the the pointer itself in that context or to the new array that was created? – T.j. Banghart May 23 '19 at 02:18
  • 1
    @Chipster Assuming that `arrayPointer` was allocated the same way (it should be, whether it was previously allocated in `add` or any other methods that resizes), then `delete [] arrayPointer` is the right way to destroy the objects and deallocate the memory. ([more details here](https://stackoverflow.com/questions/2425728/delete-vs-delete-operators-in-c/2425771)) – Gilles-Philippe Paillé May 23 '19 at 02:19
  • 1
    @T.j.Banghart There is one flaw. If `new[]` throws an exception, `sizeOfArray` now has the wrong value instead of the original value before the exception was thrown. You need to restore (or not change) the state of your object before the call to `new[]`. I'm assuming `sizeOfArray` is a member variable. – PaulMcKenzie May 23 '19 at 02:28
  • @PaulMcKenzie thanks for pointing that out -- I'm just learning exception catching so my repertoire is limited on how to do this. If I were to rearrange `T* temp = new T[sizeOfArray*2]` and then update `sizeOfArray`, would that be an adequate "catch" (not really a `catch` obviously) – T.j. Banghart May 23 '19 at 02:32
  • Yes, that would be ok, since you're not changing the state of the object before the `new[]` call. – PaulMcKenzie May 23 '19 at 02:33
  • @Giles. Wait, you're right. I get those confused. To the OP, disregard my comment. –  May 23 '19 at 06:15

2 Answers2

1

As pointed out in the comments to your post, your solution is correct.

In more detail to explain why you're correct, you made sure to allocate more memory before you copied and deleted your current data. That's the only order of things: Reserve (new array), copy, unreserve (old array). (That's how I remember it, anyway.)

In even more detail: temp is a pointer, not the array itself. This is a very crucial and often misunderstood point. I struggled with it a lot, personally. So, when you simply say T* temp;, you are allocating space for a pointer in your current frame. When you say T* temp = new T[size];, you are allocating space for a pointer in your current frame AND asking for more space (equal to sizeof(T) * size Bytes) elsewhere in memory.

This means that temp, as a pointer, is a local variable, but what it points to is not. When you assign arrayPointer = temp;, you are saying your data member points where temp points, but it is not equal to temp in the sense that it is a local variable. This is why you want to delete[] arrayPointer before assigning it to be equal to temp, otherwise you can never reclaim the memory that arrayPointer pointed to. Finally, when you say arrayPointer = temp;, nothing in the memory temp points to is copied over; only the (pointer) value of temp is copied into arrayPointer (hence why you have to explicitly copy the members of the original array into the new array, but not the other way around). Then, when your process exits that frame, all locally declared variables are released, which is why temp goes away as a pointer, but what it pointed to is not released because it was not in the frame (even though it was allocated in the frame).

A couple of pro tips, though: instead of your for-loop, I recommend taking a look at std::copy, and temp = nullptr; is actually superfluous because the memory allocated for temp (as a local variable) will be deallocated once the function returns (as discussed above).

Hopefully that helps a bit conceptually. But, again: you are definitely right :)

0

If p has value nullptr, then delete p does nothing.

If you have seen examples of people setting a raw pointer to null and then deleting it, those examples were bad code.

Setting a pointer to nullptr does nothing to the underlying memory it pointed to before — this is a key difference between C++ and most reference counted or garbage collected languages.

On the other hand, there are many “smart pointer” classes out there implementing reference-counting semantics; std::shared_ptr is the most famous one. So maybe you have seen someone using one of those. For these, typically resetting the pointer to null would decrease its reference count, causing it to be deleted if the reference count was 1. But this would be handled by the class; you wouldn’t call delete yourself.

Brennan Vincent
  • 10,736
  • 9
  • 32
  • 54