I am looking at the following piece of linked list code I found online:
void DeleteAfter(Node **head){
if(*head==NULL){
return;
}else{
Node *temp = NULL;
temp = (*head)->next;
(*head)->next = (*head)->next->next;
delete temp;
temp=NULL;
}
}
I am not that skilled with C++, so this could be a bad question, but why is temp being set to NULL after being deleted? Is this a necessary step?
It's unnecessary. Some people make a habit of doing this even when it has no result. An aggressive compiler optimizer will eliminate this code, so it doesn't actually do any harm. But I would have written:
void DeleteAfter(Node *head) {
if (head) {
Node *next = head->next;
if (next) {
head->next = next->next;
delete next;
}
}
}
Note I eliminated a useless level of indirection and added a check to make sure there's a "node after" to delete.
The rationale for the habit is that if a pointer always refers to either a valid object or is null, you can rely on null checks as equivalent to validity checks.
For this reason, Ada, a language often used in safety critical systems, initializes pointers to null and defines its delete equivalent operator to set its argument null automatically. Your C++ is simulating this behavior.
In practice, the value of this discipline is not what you'd hope. Once in a long while it prevents a silly error. One nice thing, however, is that debugger displays of pointer content make sense.
If the variable temp could possibly be used again later on in the code, then it is good practice to set it to NULL.
There are two reasons you'd normally set a pointer to NULL after releasing it.
1.) Once you release a pointer the memory at the address pointed to is no longer available to your program. Theoretically, that memory could now be used by any other program, including the operating system itself! Attempting to release a pointer that has already been released and thus points to who knows what could cause a huge problem. Luckily modern operating systems protect against this but the program will still crash with an illegal access error. Releasing a null pointer OTOH will do absolutely nothing.
2.) You should always check that a pointer isn't NULL before de-referencing it with the * operator. De-referencing a NULL pointer will cause a run-time error. De-referencing a released pointer that points to some arbitrary memory is even worse. A released pointer should always be set to NULL so the later code can assume a non-null pointer points to valid data. Otherwise there is no way of knowing whether the pointer is still valid.
As for the original question, the pointer variable temp is declared as a local variable in a short function where it is never used again. In this case it's unnecessary to set it to NULL since it goes out of scope as soon as the function returns.
However, the line...
(*head)->next = (*head)->next->next;
fails to makes sure (*head)->next is not null before attempting to de-reference, a no-no.
A better version would be...
int DeleteAfter(Node **head){
Node *node_after = NULL;
if(*head==NULL)
return -1;
node_after = (*head)->next;
if(node_after == NULL)
return -1;
(*head)->next = node_after->next;
delete node_after;
return 0;
}
Now the person using the function can check whether the node deletion was successful by the return value and there is no risk of trying to delete a non-existent node.
You dont have to set the local pointer variable to NULL after deleting it.You should set pointers to NULL if you want to reuse the pointer, after NULL checking, u can safely assign a new address to it.Normally we do it for pointer member variables and global pointer variables.
If temp was a global or a member variable, then setting to NULL isn't a bad idea.
I got in the habit of setting pointers to NULL after using a conservative garbage collector with C code. Having no pointers to unused memory is how it finds garbage to collect. But in this case you should also do
temp->next = NULL;
In your code example there is no obvious immediate benefit, however there is arguably a loner-term maintenance cost benefit. The idea is that someone could eventually add code after your deletion of temp that attempts to dereference temp. This could happen simply by not noticing the delete, or by moving earlier lines that access temp after the deletion.
Here is an example:
int * i = new int(12);
std::cout << *i << std::endl; // output is 12.
delete i;
// i = 0; // This would cause the program to fail on the next line.
std::cout << *i << std::endl; // output is random for me.
Note that this doesn't hide a defect, in fact not setting the pointer to null would, in this case, hide the defect as *i returns a random value.
Most would say i = 0 is likely optimized out by a compiler, either way an assignment to a pointer is mostly innocuous. For me, I always err on the side caution when developing professionally.
It's not necessary, and some (including me) consider it bad practice.
The motivation to setting it to NULL is that you can check afterwards whether it was deleted, and access it otherwise if it wasn't. Also, this would prevent a double delete, because delete on a NULL pointer is a no-op.
On the other hand, it can hide bugs. If the object was deleted, there's no point in using it, right? You should know that the object was deleted, not rely on a check.
For example
if (p != NULL) //or just if (p)
p->doStuff()
Why? Don't you already know if it was deleted or not? Isn't cleanup part of the logic?
