I am reading a function that deletes a node from a simple linked list from the C how to program fifth edition:
Code:
char delete( ListNodePtr *sPtr, char value )
{
ListNodePtr previousPtr; /* pointer to previous node in list */
ListNodePtr currentPtr; /* pointer to current node in list */
ListNodePtr tempPtr; /* temporary node pointer */
/* delete first node */
if ( value == ( *sPtr )->data ) {
tempPtr = *sPtr; /* hold onto node being removed */
*sPtr = ( *sPtr )->nextPtr; /* de-thread the node */
free( tempPtr ); /* free the de-threaded node */
return value;
} /* end if */
else {
previousPtr = *sPtr;
currentPtr = ( *sPtr )->nextPtr;
/* loop to find the correct location in the list */
while ( currentPtr != NULL && currentPtr->data != value){
previousPtr = currentPtr; /* walk to ... */
currentPtr = currentPtr->nextPtr; /* ... next node */
} /* end while */
/* delete node at currentPtr */
if ( currentPtr != NULL ) {
tempPtr = currentPtr;
previousPtr->nextPtr = currentPtr->nextPtr;
free( tempPtr );
return value;
} /* end if */
} /* end else */
return '\0';
} /* end function delete */
I focus to lines 24-30:
Code:
/* delete node at currentPtr */
if ( currentPtr != NULL ) {
tempPtr = currentPtr;
previousPtr->nextPtr = currentPtr->nextPtr;
free( tempPtr );
return value;
} /* end if */
I think that the line:
Code:
tempPtr = currentPtr;
that keeps a temporary copy of the currentPtr to release later the memory that this memory position has allocate is exaggeration. There is no need for this. We can refer to this position directly with currentPtr because currentPtr's value does not change from the statement before free() function. So I think that this is a better way:
Code:
/* delete node at currentPtr */
if ( currentPtr != NULL ) {
previousPtr->nextPtr = currentPtr->nextPtr;
free( currentPtr );
return value;
} /* end if */