problems in creating copy constructor for linkedlist in c++

[loveranjan]

#include <cstdlib>
#include <iostream>
#include <conio.h>
#include <new>

using namespace std;

class Array {
    struct node {
        char *str;
        int item;
        struct node *next;
    }*p;
    public :
        Array()
        {
            p = NULL;
        }

        ~Array()
        {
            node *s;
            
            if(p == NULL)
                return;
                
            while(p != NULL)
            {
                s = p->next;
                delete p->str;
                delete p;
                p = s;
            }
        }

        void getValue(int i)
        {
            int k=0;
            node *s = p;
            
            while(s != NULL)
            {
                if(k == i)
                    cout << s->str << " " << s->item << endl;
                s = s->next;
                k++;
            }
        }

        void putValue(int j)
        {
            node *s,*t;
            if(p == NULL)
            {
                t = new node;
                t->str = new char[20];
                t->item = j;
                t->str = "Hello";
                t->next = NULL;
                p = t;
            }
            else 
            {
                t = p;
                while(t->next != NULL)
                    t = t->next;
                s = new node;
                s->str = new char[20];
                s->item = j;
                s->str = "Hello";
                s->next = NULL;
                t->next = s;
            }
        }
        void display()
        {
            node *t;
            t = p;
            while( t != NULL)
            {
                cout << t->str << " " << t->item << " ";
                t = t->next;
            }
        }
        Array(const Array &a);
};

Array::Array(const Array &a)
{
    node *s,*t,*r;
    s = a.p;
      
    while(s != NULL)
    {
        if(t == NULL)
        {
            t = new node;
            t->str = new char[20];
            t->item = s->item;
            t->str = "Priya";
            t->next = NULL;
            p = t;
        }
        else 
        {
            t = p;
            while(t->next != NULL)
                t = t->next;
            r = new node;
            r->str = new char[20];
            r->item = s->item;
            r->str = "Priya";
            r->next = NULL;
            t->next = r;
        }
        s = s->next;
    }
    cout << "copy done";
}

int main()
{
    Array num;
    int i;
    
    for(i=0;i<10;i++) num.putValue(i);
    for(i=9;i>=0;i--) num.getValue(i);
    cout << "\n";

    Array x(num);
    x.display();
    for(i=9;i>=0;i--) x.getValue(9-i);
    
    getch();
    return 0;
}

[drkybelk]

Can you describe the problem you got?

I think you have a serious problem, if I understand your code correctly. I haven't compiled or run it, but your nested class node will cause grief and bodily harm, I believe.
A struct is exactly the same as a class in C++ (with the only difference being the default access modifier being public: in structs). Hence all the cautions and precautions that you implement in classes should also be implemented in structs.
Your node class class will have the DEFAULT copy constructors, assignment operator etc. This is dangerous in your case and most likely the problem you ae talking about. You have members in node that are pointers and as a strong rule of thumb you should always implements the following 4 methods if you have pointer members:
- default constructor
- copy constructor
- assignment operator and
- destructor.
The reason for this is that the default ones only provide flat copy and that is particularly bad in the case of the copy constructor in combination with the destructor, because more likely than not your code will try to delete objects multiple times.

struct xxx
{
   char* data;
};

void f(xxx x)
{
}

int main()
{
   xxx x1;
   x1.data = new char[20];
   f(x1);   // flat copy of x1 passed to f
}// flat copy of x1 and x1 itself are deleted which means 
// x1.data and (x1 copy).data are to be deleted, but they both point to the same memory address
// ----> crash

[loveranjan]

The problem here I am facing is that when I create struct (member of Array class) as
struct node { char *str;int item;}*p; instead of struct node { int str;int item; }*p; then it is creating problem in creating copy constructor.I have also attached the correct code
( named as copyConstructorNew.cpp ) with new struct member which give correct answer.So, problem here is only to deal with pointer struct member i.e. char *str

Here is new code for copy constructor for linked list with different structure member.

#include <cstdlib>
#include <iostream>
#include <conio.h>
#include <new>

using namespace std;

class Array {
    struct node {
        int str;
        int item;
        struct node *next;
    }*p;
    public :
        Array()
        {
            p = NULL;
        }

        ~Array()
        {
            node *s;
            
            if(p == NULL)
                return;
                
            while(p != NULL)
            {
                s = p->next;
                delete p;
                p = s;
            }
        }

        void getValue(int i)
        {
            int k=0;
            node *s = p;
            
            while(s != NULL)
            {
                if(k == i)
                    cout << s->str << " " << s->item << endl;
                s = s->next;
                k++;
            }
        }

        void putValue(int j)
        {
            node *s,*t;
            if(p == NULL)
            {
                t = new node;
                t->item = j;
                t->str = 10;
                t->next = NULL;
                p = t;
            }
            else 
            {
                t = p;
                while(t->next != NULL)
                    t = t->next;
                s = new node;
                s->item = j;
                s->str = 10;
                s->next = NULL;
                t->next = s;
            }
        }

        void display()
        {
            node *t;
            t = p;
            while( t != NULL)
            {
                cout << t->str << " " << t->item << " ";
                t = t->next;
            }
        }
        Array(const Array &a);
};

Array::Array(const Array &a)
{
    node *s,*t,*r;
    s = a.p;
      
    while(s != NULL)
    {
        if(t == NULL)
        {
            t = new node;
            t->item = s->item;
            t->str = 20;
            t->next = NULL;
            p = t;
        }
        else 
        {
            t = p;
            while(t->next != NULL)
                t = t->next;
            r = new node;
            r->item = s->item;
            r->str = 20;
            r->next = NULL;
            t->next = r;
        }
        s = s->next;
    }
    cout << "copy done\n";
}

int main()
{
    Array num;
    int i;
    
    for(i=0;i<10;i++) num.putValue(i);
    for(i=9;i>=0;i--) num.getValue(i);
    cout << "\n";

    Array x(num);
    for(i=9;i>=0;i--) x.getValue(9-i);
    
    getch();
    return 0;
}

[drkybelk]

What I said in my previous still needs sorting out in your code, but there is also another problem.

t = new node;
            t->str = new char[20]; // 1.
            t->item = s->item;
            t->str = "Priya";      // 2.
            t->next = NULL;
            p = t;

This is *NOT* what you actually want. You create an array of 20 new characters and assign them to your pointer str in line 1. In line 2. you re-assign the pointer to have the address of the constant character array "Priya". You lost all reference to the newly created 20 characters effectively creating a memory leak. Worse yet: Had you created a proper destructor for your node class then you would eventually try to delete this constant string, which would make your program crash.
What you want to do here is to use strcpy() function to copy the string "Priya" into your allocated 20 characters.

strcpy(r->str,"Priya");

or sth like that.
But you should also not forget to create the 4 methods as stated in my previous post.