Using Destructor to deallocate linklist

Question

So I am using linklist to make a list of names and i have been given a task to use both this pointer as well as destructor.The code written by me is

#include <iostream>
using namespace std;

class node
{
    char name[50];
    node*next;
    public:
    friend class sll;

};
class sll:public node
{
    public:
    node*head;
    node*last;
    int*k;
    friend class node;
    sll()
    {
        head=NULL;
        last=NULL;
    }
    ~sll()
    {
        node*temp2;
        node*temp;
        temp = head;
        while(temp!=NULL)
        {
            temp2 = temp->next;
            delete(temp);
            temp = temp2;
        }
    }
    void input()
    {

        node*temp;
        temp = new node;
        cout<<"Enter Name"<<endl;
        cin>>temp->name;
        if(head==NULL)
        {
            head = temp;
            last = temp;
        }
        else 
        {
            last->next = temp;
            last = temp;
        }

    }
    void output()
    {
        node*temp;
        temp = head;
        while(temp!=NULL)
        {
            cout<<temp->name<<" ";
            temp = temp->next;
        }
    }
    sll &get()
    {
        return *this;
    }
};
int main()
{
    sll obj,obj1;
    obj.input();
    obj1 = obj.get();
    obj1.output();
    return 0;
}

The error i am getting is double free or corruption Now what i think is that i am getting this particular error because obj1 = obj.get()copies the address of obj in obj1. Hence when i try to delete the linklist ,it gives me error because i am freeing in twice(as i have two objects of class sll).But if it is so, why am i not getting any error if i replace the destructor block with

~sll()
{
   delete(k);
}

Can Anyone help me to figure it out?

`delete` doesn't use parentheses, so your code should look like this `delete k;` I have no idea why you want to delete `k` or why you make a base class/derived class a friend. Are you using c++11, c++14 or c++17? — An0num0us, Aug 23 '18 at 19:53
As i said i am given a task to use destructor.So as I am getting error deallocating linklist,i am trying to use destructor for some other purpose — P.Bendre, Aug 23 '18 at 19:56
Possible duplicate of [What is The Rule of Three?](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) — Alan Birtles, Aug 23 '18 at 20:01
`obj1 = obj.get()` makes a copy but you haven't implemented the rule of 3/5 — Alan Birtles, Aug 23 '18 at 20:03
I.e you now have *two* lists harboring the same pointers Consider what happens to the validity of the pointers in one when the other is destroyed. Unrelated, I'm trying to fathom the reason for the `get()` method, and I honestly can't think of one good reason to do that. — WhozCraig, Aug 23 '18 at 20:06
Once you have corrected the errors. You should take the code to https://codereview.stackexchange.com (they review working code) but will tell you how to make the code better using best practices. — Martin York, Aug 23 '18 at 20:08

Christophe · Accepted Answer · 2018-08-23T22:52:22.130

The problem

The statement, copies obj into obj1:

obj1 = obj.get();

Unfortunately, your class has assignment operator. So the default one is used, which makes a member by member copy. Unfortunately, all the pointers will be copied exactly with their same value. In consequence obj.head and obj1.head both point to the same object.

When both linked list get destroyed at the end of main(), the first one destroyed managed to destroy all its nodes. But this means that all the pointer in the remaining list become dangling. When the second list is destroyed, this creates of course an issue of trying to delete something that no longer exists !

The solution

THis can be solved by implementing an assignment operator, that constructs the new list by cloning its nodes and pointing to the clone:

sll& operator=(const sll& o)   // assignment operator
{
    node *temp;      // iterate through original container
    temp = o.head;
    while(temp!=nullptr)
    {                // for every node of the list create a clone
        node *clone = new node(*temp); 
        if(head==nullptr)  // add it to the new list
        {
            head = clone;
            last = clone;
        }
        else 
        {
            last->next = clone;
            last = clone;
        }
        temp = temp->next;
    }
    return *this; 
}

Test demo

But this is not sufficient, because similar issues occur when you're doing a copy construction. So you'd also need a copy constructor.

As soon as you have pointers in a class, you should think to apply the rule of 3 to avoid such cases

But why am I not getting any error if I delete k in destructor.I mean it also gets copied in both the pointers and hence pointed by both the pointers. — P.Bendre, Aug 24 '18 at 03:32
@P.Bendre deleting an already deleted pointer is undefined behavior. So it can crash but it doesn't have to. If k was nullptr, delete would have no effect. If you don't delete the nodes, your programme will leak. — Christophe, Aug 24 '18 at 05:57

Using Destructor to deallocate linklist

1 Answers1