Well, good practice would be to not create redundant values.
That being said, it's more than "good practice" to release resources you allocate, though, mainly because if you don't your program is horribly broken --- leaking memory is not a "minor" issue.
On some platforms (on anything freestanding, for example) large-enough memory leaks will cause the system to catastrophically fail. On others, Linux, for instance, it's likely (but not guaranteed) that the kernel will send SIGKILL to your program, crashing it, after causing the system to slow to a crawl while your software consumes any swap space the system has available. If the kernel doesn't kill your program, it kills another.
That's not a valid preference. If you're using pointers
because you like the notation, you're introducing complexity for no reason and making your code significantly more error prone by using pointers. This is especially the case if you're using
new
unnecessarily, which isn't a good idea because it forces the programmer to take into account resource management for no reason.
LinkedList<int> *llist2 = new LinkedList<int>(lnode); is meant for cases where I have to instantiate the list with a pre-created node (as illustrated in my main.cpp's construct 2). |
What's wrong with
LinkedList<int> llist2(lnode);
?
Note that in push_back, you're reassigning
nodePtr
after allocating dynamic memory for it.
Once you lose the reference to that dynamically-allocated ListNode, it's leaked. You don't need it.
If you had to do that, you must
delete
the resource stored at that pointer before you lose your last reference to it, and once it's deleted, you must never access that resource again.
Let's slightly modify push_back:
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31
|
template<class T>
void LinkedList<T>::push_back(T newData)
{
// dynamically allocate this one --- if it had automatic storage
// duration, it would be destroyed upon returning from the function.
ListNode<T> * const newNode = new ListNode<T>;
newNode->data = newData; // Maybe do this in the node ctor?
// If there are no nodes in the list
// make newNode the first node.
if (!head)
{
head = newNode;
head->next = nullptr;
}
else // Otherwise, insert newNode at end.
{
// Initialize nodePtr to head of list.
ListNode<T> const *iter = head;
// Find the last node in the list.
while (iter->next)
{
iter = iter->next;
}
// Insert newNode as the last node.
iter->next = newNode;
newNode->next = nullptr;
}
}
|
There's no
delete
, because you need the node you dynamically created to exist in the list until it is removed or the list node is otherwise destroyed (probably along with the list it's contained in.)