Linked list and overloaded assignment operator

[MatthiasE]

I am trying to overload the Assignment and Copy operators for a simple linked list class. It seems I can only copy the pointers, when I delete the original list, my program has errors.

Here's the code:

#include <iostream>
using namespace std;

struct link
{
int data;
link* next;
};

class linklist
{
private:
link* first;
public:
linklist()
{ first = NULL; }
linklist(linklist& a)
{ *this = a; }
~linklist()
{
link* current = first;
while(current != NULL)
{
link* temp = current;
current = current->next;
delete temp;
}
}
void additem(int d)
{
link* newlink = new link;
newlink->data = d;
newlink->next = first;
first = newlink;
}
void display()
{
link* current = first;
while(current != NULL)
{
cout << endl << current->data;
current = current->next;
}
}
void deleteitem()
{
link* temp = first->next;
delete first;
first = temp;
}
linklist& operator = (linklist &a)
{
if(this==&a)
return *this;

while(first!=NULL)
deleteitem();
link* current = a.first;
while(current!=NULL)
{
additem(current->data);
current=current->next;
}
first = current;
return *this;
}
};

int main()
{
linklist* li = new linklist;
li->additem(25);
li->additem(36);
li->additem(49);
li->additem(64);
li->display();

linklist* lj = li;
//lj = li;
delete li;
cout << endl << endl;
lj->display();
cout << endl;
return 0;
}

 

[timmay3141]

Your problem seems to be with the line "first = current" in your = operator. You are in the while loop until current = NULL, so here you set first = NULL, which is certainly not what you intend to do.

 

[uolj]

Code tags would make that easier to read...

linklist(linklist& a)
{ *this = a; }

This copy constructor is trying to call the assignment operator. You can't do that, because you haven't fully constructed the "this" object yet. You need to copy the data inside a, kind of like you do inside your assignment operator (in fact, technically that code should be in the copy constructor, and then you could implement your assignment operator in terms of your copy constructor and a swap).

 

[timmay3141]

Also, I didn't look at the code in main() when I posted first, but think about what you are doing. You are dynamically allocating a linkedlist and setting a pointer li to that memory location, then declaring another pointer and assigning it to the same thing, lj. Of course this won't copy the linked list, because you aren't telling it to, you are just assigning a pointer to the same memory location.

 

[MatthiasE]

Right...and I mean to copy the data, not just a pointer to it, through the overloaded assignment operator, i.e.creating a whole new linked list with the exact same contents as list 1.

 

[timmay3141]

Then you should do this.

int main() 
{ 
  linklist li; 
  li.additem(25); 
  li.additem(36); 
  li.additem(49); 
  li.additem(64); 
  li.display(); 

  linklist lj = li; 
  // linklist should probably have a Clear() method, which
  // you should call here to make sure it works
  cout << endl << endl; 
  lj.display(); 
  cout << endl; 
  return 0; 
}

When you have pointers and use =, you are merely reassigning a pointer, not the actual thing being pointed to. Another way of doing this would be to dynamically allocate lj, then say "*lj = *li".