Is this a correct use of delete?

Did I use delete correctly in this code snippet?
1
2
3
4
5
6
7
void pop(Stack *&top){
        if(top == nullptr) return;
        Stack *temp = top;
        temp = top->next;
        delete top;
        top = temp;
    }


It works fine but idk if it's luck or it was actually used correctly, i was thinking.. I want to get rid of top's current node then assign it to temp later.
Yes, it looks correct under the assumption that the Stack object pointed to by top has been allocated with new and the destructor does not delete the whole stack recursively.
Last edited on
@Peter87, I was just confused when I wrote
1
2
delete top;
        top = temp;

Cause I was like.. this looks wrong then it worked after testing so its kind of confusing me, doesn't delete operator destroy the 'top' pointer? I thought re-assigning it after delete operator would result in an error either syntax or logical, even though no error occurs.
Last edited on
delete doesn't destroy the pointer. It destroys what the pointer points to.
OP: this link might interest you – code-review for a class template implementation of Stack with std::shared_ptr:
https://codereview.stackexchange.com/questions/163751/implementing-a-stack-with-templates-and-smart-pointers
The initial code posted is augmented with further comments from users and I tried adding a vocal dtor to the Node class to check if the memory was being released properly and it seems to be: ~Node() {std::cout << "goodbye " << data << "\n";}
Using smart pointers there'd be no need to call delete but you need to make sure that any changes that you make of your own also continue to acquire and release resources properly
Your delete is fine (with the above assumptions from @Peter87), but you confuse things a little it with the way you assign things. Assigning temp = top (line 3), then temp = top->next (line 4) is an extra step that confuses things a bit. In order to make things a little cleaner, I would go one of the following 2 routes:

1
2
3
Stack *temp = top->next;
delete top;
top = temp;


1
2
3
Stack *temp = top;
top = top->next;
delete temp;


I would probably use the second approach. I could give semantic arguments for why it is a slightly better approach, but at some level it boils down to a personal preference.
Topic archived. No new replies allowed.