Copy Linked List

[AdventGamer]

I've been working with linked list and i'm currently trying to make my copyList template function work. It should return the head of the new list. Could someone give me some advice on how to fix it, currently i get no output when i try to print the copied list.

#include <iostream>
#include <list>
#include "node.h"
#include <string>

using namespace std;

template <typename T>
node<T> *copyList(node<T> *front)
{
	//set tempHead to front's nodeValue, not sure if this is correct.
	node<T> *tempHead = new node<T>(front->nodeValue);
	node<T> *temp;

	//iterate through list and copy to newly allocated memory
	while(front->next != NULL)
	{
		//create new node
		temp = new node<T>;
		//set newnode equal to tempHead's next.
		temp = tempHead->next;
	}

	//should return the head of the new list but i think
	//there is something wrong here.
	node<T> *temp2 = tempHead;
	return temp2;
}

template <typename T>
void writeLinkedList(node<T> *front, const string& separator = " ")
{
	//front points at first node. curr moves through the list
	node<T> *curr;
	curr = front;

	while (curr != NULL)
	{
		//output node value and move to the next node
		cout << curr->nodeValue << separator;
		curr = curr->next;
	}
}

int main()
{
	//create list and add items to front
	node<double> *front = NULL, *p, *newHead = NULL;
	int i = 0, n = 0;

	p = new node<double>(5.5, front);
	front = p;
	p = new node<double>(6.7, front);
	front = p;
	p = new node<double>(15.3, front);
	front = p;
	p = new node<double>(3.14, front);
	front = p;
	p = new node<double>(2.718, front);
	front = p;
	p = new node<double>(15.3, front);
	front = p;
	p = new node<double>(3.5, front);
	front = p;

	//print out lists
	cout << "List One: " << endl;
	writeLinkedList(front, " ");
	cout << endl << endl;

	cout << "Copied List: " << endl;
	newHead = copyList(front);
	writeLinkedList(newHead, " ");
	cout << endl << endl;

	system("PAUSE");
	return(0);
}

node class:

template <typename T>
class node
{
public:
	T nodeValue;	//data held by the node
	node<T> *next;	//next node in the list

	//default contructor with no initial value
	node() : next(NULL)
	{}


	//constructor. initialize nodeValue and next
	node(const T& item, node<T> *nextNode = NULL) :
		nodeValue(item), next(nextNode)

	{}
	
};

[mrnutty]

Your problem is here first :

//iterate through list and copy to newly allocated memory
	while(front->next != NULL)
	{
		//create new node
		temp = new node<T>;
		//set newnode equal to tempHead's next.
		temp = tempHead->next;
	}

First, use the condition while( front != NULL) {...}

Second, your logic is not correct. You need to set tempHead->next equal to temp; Not the other way around.
You also have not initialize temp, to hold the value needed. And you also need to save a copy of the tempHead so you
can return it later.

[mike_2000_17]

Did you consider making a method instead? such as:

template <typename T>
class node
{
public:
	T nodeValue;	//data held by the node
	node<T> *next;	//next node in the list

	//default contructor with no initial value
	node() : next(NULL)
	{}


	//constructor. initialize nodeValue and next
	node(const T& item, node<T> *nextNode = NULL) :
		nodeValue(item), next(nextNode)

	{}
	
        node<T>* clone() {
                if(next)
                        return new node<T>(nodeValue,next->clone());
                else
                        return new node<T>(nodeValue,NULL);
        }; 
};

... in main():
	cout << "Copied List: " << endl;
	newHead = front.clone();
	writeLinkedList(newHead, " ");
	cout << endl << endl;
...

[AdventGamer]

thank you both for the help, and as for mike's post I can't really do that on this project, but it is nice to see alternatives. And for firstPersons post I'm pretty confused. Below is what I have so far, still working on figuring this out.

template <typename T>
node<T> *copyList(node<T> *front)
{
	//set tempHead to front's nodeValue, not sure if this is correct.
	node<T> *tempHead = new node<T>(front->nodeValue);
	node<T> *temp;
	node<T> *tempHeadCopy;

	//iterate through list and copy to newly allocated memory
	while(front != NULL)
	{

		//create new node and assign it the value of tempHead's nodeValue
		temp = new node<T>(tempHead->nodeValue);
		//set tempHead's next equal to temp
		tempHead->next = temp;
	}

	return tempHeadCopy;
}

[AdventGamer]

ok after some book reading and googling, i came up with some changes that to me look correct but the compiler thinks differently. Can anyone spot a problem? When it runs i'm getting

Unhandled exception at 0x01022646 in Joust06AJBO.exe: 0xC0000005: Access violation writing location 0x00000000.

template <typename T>
node<T> *copyList(node<T> *front)
{
	node<T> *tempHead = front;
	node<T> *newList = NULL;
	node<T> *tail = NULL;

	while (tempHead != NULL)
	{
		if (newList == NULL)
		{
			newList->nodeValue = tempHead->nodeValue;
			newList->next = NULL;
			tail = newList;
		}
		else
		{
			tail = tail->next;
			tail->nodeValue = tempHead->nodeValue;
			tail->next = NULL;
		}
		tempHead = tempHead->next;
	}
	return (newList);
}

[mrnutty]

@OP : You need to do something like this :

template<typename T>
node<T>* copyList(node<T>* front){
 node<T> *newHead = new node<T>(front-value);
 node<T> *curr = newHead;
 for(node<T>* pos = front->next; pos != NULL; pos = pos->next){
    node<T> *tmp = new node<T>(pos->value);
    curr->next = tmp;
    curr = curr->next;
 }

 return newHead;
}

Its not tested so no guarantees.

[AdventGamer]

thanks it works, guess i was way closer to begin with.

[mike_2000_17]

With Linked-list, recursion is your best friend:

template<typename T>
node<T>* copyList(node<T>* front){
 if(front)
   return new node<T>(front->value,copyList(front->next));
 else
   return NULL;
};

[mrnutty]

>>With Linked-list, recursion is your best friend:

I have to disagree, there is no need for recursion here.