linked list copy constructor

[nightmaregiba]

is my copy constructor for linked list right??

LINKED_LIST<T>::LINKED_LIST(const LINKED_LIST &aList)
{
	ListNode *p1 = head;
	ListNode *p2 = aList.head;
	for (unsigned i = 0; i < aList.size)
	{
		p1->data = p2->data;
		p1 = p1->next;
		p2 = p2->next;
	}

}

template <class T> 
class LINKED_LIST
{
private:

	struct ListNode
	{
		T data; 
		ListNode *next; 
	};

	unsigned int size; 
	ListNode *head; 

public:

	LINKED_LIST(); 
	LINKED_LIST(const LINKED_LIST &aList); // copy constructor
}
.....
.....

thks

[coder777]

you make a shallow copy of the list. When list1 destroys an element list2 will still reference it and will probably crash later. So no it's not corrrect.

You need to create a new ListNode for each element in aList

[nightmaregiba]

here is insert after index

int LINKED_LIST<T>::insert(unsigned int index, T newItem)
{
	ListNode *pNewNode = new ListNode;
	pNewNode->data = newItem;
	pNewNode->next = NULL;

	if (index > size) return 0; // fail
	else 
		if (index == 0) // addhead
		{
			if (head == NULL) 
			{
				head = pNewNode;
			}
			else
				{	
					pNewNode->next = head;
					head = pNewNode;
					size++;
				}
			
		}
		else
			{
				ListNode *p = head;
				unsigned pos = 0;
				while (pos != index - 1)
				{
					p = p->next;
					pos++;
				}

				pNewNode->next = p->next;
				p->next = pNewNode;
				size++;
			}
	return 1; // succeed
}

is it correct?

thks

[nightmaregiba]

it not run, i dont know why>

[dhayden]

Your copy constructor needs to update size also. If there are other constructors, make sure they do too,

insert() needs to update size at line 13.

[nightmaregiba]

thks

[nightmaregiba]

Here is my code
but still not working

template <class T>
LINKED_LIST<T>::LINKED_LIST(const LINKED_LIST &aList)
{
	ListNode *p = aList.head;

	//create first node
	ListNode *q = head;
	q->data = p->data;
	q->next = NULL;
	size++;
	p = p->next;

	for (unsigned int i = 2; i <= aList.size; i++)
	{
		T newItem = p->data;
		p = p->next;
		insert(i,newItem);
		size++;
	}

}

[dhayden]

This is a constructor. That means that upon entry, head and size contain garbage and must be initialized before you can use them.

[dhayden]

This is a constructor. That means that head and size contain garbage on entry and must be initialized. Line 7 and 10 assume that they contain something valid.

[nightmaregiba]

Here is insertion

PLZ check for me

int LINKED_LIST<T>::insert(unsigned int index, T newItem)
{
	ListNode *pNewNode = new ListNode;
	pNewNode->data = newItem;
	pNewNode->next = NULL;

	if (index > size) return 0; // fail
	else 
		if (index == 0) // addhead
		{
			if (head == NULL) 
			{
				head = pNewNode;
				size++;
				return 1;
			}
			else
				{	
					pNewNode->next = head;
					head = pNewNode;
					size++;
					return 1;
				}
			
		}
		else
			if (index == size) //addtail
			{
				ListNode *p = head;
				unsigned int pos = 0;
				while (pos != index - 1)
				{
					p = p->next;
					pos++;
				}
				p->next = pNewNode;
				size++;
			}
			else
				{
					ListNode *p = head;
					unsigned int pos = 0;
					while (pos != index - 1)
					{
						p = p->next;
						pos++;
					}

					pNewNode->next = p->next;
					p->next = pNewNode;
					size++;
					return 1;
				}
	return 0;
}

[doug4]

Generally the code looks pretty good. I do have a couple of comments, though:

Line 7 is a memory leak. You have allocated pNewNode, and then, if your index is out of range, you return without deleting it.

The last element in the list has next = NULL. There is no harm reassigning the this value in pNewNode if it happens to be inserted at the end of the list. So, you can completely get rid of lines 27 - 39.

Edit: Oh yeah. Why return an int? A bool indicating success or failure might be cleaner.