Copy Constructor Being Called on the Wrong Object

Question

I'm currently implementing a copy constructor for a Linked List class. When I create a new instance of the class with another Linked List as a parameter, the constructor is being called for the object I'm passing as the parameter. This is leaving me confused beyond belief. Here is the section necessary to understand what's going on in the main method:

int main()
{
   LinkedList ll;
   LinkedList ll2(ll);
}

So, instead of calling the copy constructor for ll2, the copy constructor for ll is being called. I've confirmed that the size of ll is correctly 3 before I attempt to copy ll into a new LinkedList, namely ll2. After the copy though, both have the same size, greater than 3, but even more weird, the copy constructor of ll is being called, not that of ll2. Since I'm using VC++, I've stepped through the program to confirm this.

Here is the copy constructor for the LinkedList class:

LinkedList::LinkedList(const LinkedList & other)        
{
   LLNode *otherCurNode = other.GetFirst();
   if (otherCurNode != NULL)
   {
      front = new LLNode(otherCurNode->GetValue(), NULL, NULL);
      back = front;
   }
   else
   {
      front = NULL;
      back = NULL;
   }
   LLNode *curNode = front;
   while (otherCurNode != NULL)
   {
      Insert(otherCurNode->GetValue(), curNode);
      curNode = curNode->GetNext();
      otherCurNode = otherCurNode->GetNext();
      back = curNode;
   }
   numNodes = other.GetSize();
}   

My apologies if this ends up being a simple problem - I'm fairly new to C++. Any help will be greatly appreciated!

Answer 1

LinkedList ll = LinkedList();

This creates a linked list instance, and this instance is then copy constructed. This looks like a Java or C#-ism. It is actually equivalent to:

LinkedList ll(LinkedList());

To create an empty linked list, simply write:

LinkedList ll;

This will implicitly call the default constructor.

Also, make sure that you do have a default constructor which properly initializes the linked list to empty. If you don't have one then the list's variables will end up with whatever garbage values are on the stack.

Answer 2

Weird things are often a sign of improper memory handling. I can see one immediate problem with the code you posted and there may be similar problems in other functions.

On the line while (otherCurNode->GetNext() != NULL), bad things will happen if otherCurNode is already NULL. This is the case both when the other list is empty as well as when you reach the end of the list via otherCurNode = otherCurNode->GetNext();. You really want to make it while (otherCurNode != NULL).

Answer 3

So, instead of calling the copy constructor for ll2, the copy constructor for ll is being called, and ll2 is being set to the same reference as ll.

You can make sure this is not what happening. Your ll and ll2 variables (BTW you can see why using lowercase l in short names is never a good idea) are allocated on the stack, they are not references. To see this, open Quick Watch while both variables are in scope, and type &ll, and then &ll2. You will see they have different addresses.

What do your default constructor and assignment operator look like? Are there any other constructors? Any other places where you assign front instance variable?