Trying to make LinkedList "copy-safe"

[incremental]

I'm trying to make a linked list I have created copy safe. This means that I'm trying to get a working copy constructor and assignment operator. I keep trying to copy the node from the original into the copy linked list I have but I get an access violation crash. Can someone eyeball my method I have right now and see what I'm doing wrong?

I know that some of what I have is wrong. Please don't focus on not returning the right thing or anything else. Right now I'd like to focus on getting the copy right. I'm leaking memory in my program right now and just need to get it copy safe.

LinkedStack::LinkedStack( const LinkedStack& original )
{
	LinkedStack copy;
	LinkedStack traveller = original; 
	while(traveller.head->next != NULL)
	{
		copy.head->data = original.head->data;
		traveller.head = traveller.head->next;
	}
	return;
}

[ne555]

LinkedStack traveller = original; calls the copy constructor (the one you are coding)

Your loop makes no sense. You copy 'n' times the first cell.

[incremental]

I was just trying to find a way to traverse the list. That's what "traveller" was for. I couldn't figure out how to iterate the loop.

[ne555]

Traverse a list

class list{
private:
  struct node{
    T data;
    node *next;
  };
  node head;
};

//Using an empty header cell. The iterator points to the cell before the data
node *iterator = &List.head;
while( iterator->next ){
  //do what you want with your data
  //iterator->next->data
  iterator = iterator->next;
}

¿How is your list constructed?

[incremental]

class stackNode
{
public:
	double data;
	stackNode * next;

	stackNode() {data = 0; next = NULL;}
	stackNode( double value ) { data = value; next = NULL; }
};

class LinkedStack
{
public:
	LinkedStack() { head = NULL;} // default constructor
	~LinkedStack(); // default destructor
	LinkedStack( const LinkedStack& );// copy constructor
	//LinkedStack& operator=( const LinkedStack& ); //assignment operator

	void Print(); // print what is stored on the stack
	bool Empty(); // return true if the stack is empty 
	int Size( int& ); // return number of elements on the stack
	double Top(); // return the top element on the stack
	void Push( int ); // add an element to the top of the stack 
	void Pop(); // remove the element on top of the stack

private:
	stackNode * head;
};

LinkedStack::~LinkedStack()
{
	//call the remove to filter through and delete from NULL up.
	while(head != NULL)
		Pop();
	delete head;
}

LinkedStack::LinkedStack( const LinkedStack& original )
{

}

//LinkedStack& LinkedStack::operator=( const LinkedStack& original)
//{
//	return *this;
//}

void
LinkedStack::Print()
{
	if(head == NULL)
	{
		cout << "Called print. " << "Stack is empty." << endl;
		return;
	}

	stackNode * current = head;
	while( current != NULL )
	{
		cout << current->data << endl;
		current = current->next;
	}
	cout << endl;
}

bool
LinkedStack::Empty()
{
	if(head == NULL)
		return true;
	return false;
}

int
LinkedStack::Size( int &LSS )
{
	stackNode * current = head;
	while(current != NULL)
	{
		LSS++;
		current = current->next;
	}

	return LSS;
}

double
LinkedStack::Top()
{
	if(head != NULL)
		return head->data;
	else
	{
		return -1.1111;
	}
}

void
LinkedStack::Push( int value )
{
	if( head == NULL )
	{
		head = new stackNode( value );
		cout << "Stack: " << endl;
		Print();
		return;
	}
	
	if(head != NULL)
	{
		stackNode * push = new stackNode( value );
		stackNode * temp = head;
		head = push;
		head->next = temp;
		temp = NULL; push = NULL;
		delete temp; delete push;
		cout << "Stack: " << endl;
		Print();
		return;
	}	
}

void
LinkedStack::Pop()
{
	if(head == NULL)
	{
		delete head;
		return;
	}

	stackNode * temp;
	temp = head->next;
	head = temp;
	temp = NULL;
	delete temp;
	cout << "Popped the top." << endl << "Stack:" << endl;
	Print();
}

This?

EDIT: Also I think the way I have it set up now I don't need to worry about memory leaks. That's the whole reason I'm trying to create the copy and assignment op methods.

[ne555]

Deleting a null pointer does nothing.
You are already traversing in Print() and Size() ¿what's your issue?

[incremental]

My issue is that I may be leaking memory. If I'm leaking memory then I need to find out where and solve the issue with it. Also, what you said about deleting a NULL pointer. Does that mean if its NULL I don't need to worry about deleting it?

[ne555]

I know that you need to implement the copy constructor (and assignment operator).
What I'm asking is why can't you do it.

LinkedStack::LinkedStack( const LinkedStack& original )
{
	if(original.empty()){ //nothing to do
		head=NULL; //as the default constructor
		return;
	}
	head = new stackNode( original.head->value );
	stackNode *ours = head, theirs = original.head;
	while(theirs->next != NULL){ // ¿do you realize why it is asking for next instead of itself?
		//copy the nodes (you need to allocate them)
		//increment the iterators
	}
}

Does that mean if its NULL I don't need to worry about deleting it?

¿Why do you delete pointers? NULL is an invalid address.

[incremental]

1) I have continuously had a hard time with pointers, more specifically with cleaning up after them. I've had a rough time understanding how to write them. Their syntax drives me up the wall.

EDIT: I know that what the copy constructor does is copy from the const object you passed in into a new object. What you need to do for a linked list is while ->next is not NULL, you need to copy the data from the original to the new list. The reason I can't write it myself is because I don't get the syntax right **ever**.

2) I was always under the impression that if you have a pointer that allocates memory when you call the return line at the end of your program that it needs to be deleted so you don't have memory leaks.

[ne555]

1_ You are missing the point.
You need to copy all the list, you know that you finish when you reach NULL.
¿So why theirs->next != NULL instead of theirs != NULL ?
Because I've started at the head. (¿is that a good reason?)

You can easily traverse the other list. The problem here is to link the cells.
Suppose a list of 3 elements.
_ Fist you copy the heads. this->head = new stackNode( other.head->value );
_# Get to the next element stackNode *theirs = other.head->next;
_ Copy that stackNode *aux = new stackNode( theirs->value );
_ And link (¿can you do it?)
_ If you didn't finish get back to #

2)

Bjarne Stroustrup wrote:

Code that creates an object using new and then deletes it at the end of the same scope is ugly, error-prone, and inefficient.

You need to delete it when you end using it (the object).

The memory leak occurs when you lose the memory address of an allocated object.
By instance

ptr = new derived;
ptr = new derived; //we lose the previous object, leak
ptr = NULL; //and again 

A NULL pointer can't be pointing to allocated memory, so lines 86-87 are superfluous (conceptual error).