C++

Forum

 
 
Linked List Copy Constructor troubles.

btripp (160)
Hey,

Im working on adding some operators to my linked list class. I did the += operator and works fine. However im attempting to make a copy construtor but im not getting any output. I dont have any errors but when after i do the copy in main for example

List linkedlist1;// i stored like 10 numbers in this list

List x(linkedlist1);

when i call
linkedlist1.print(); // works just fine

x.print();// prints nothing on the screen



here is the code for the copy constructor

1
2
3
4
5
6
7
8
9
10
11
 
List::List(const List& aList)
{
	m_first = aList.m_first;
	ListNode* pTmp = m_first;
	for(int i = 0; i < aList.get_count(); i++)
	{
		Add_To_Back(pTmp->m_element);
		pTmp = pTmp->m_next;
	}
}

here is the default constructor
1
2
3
4
5
 
List::List()
{
	m_first = NULL;
	m_count = 0;
}


here is my add to back
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
 
void List::Add_To_Back(const int &e)
{
	if(m_first == NULL)
		Add_To_Front(e);
	else
	{
		m_count++;
		ListNode* pTmp = m_first;
		while(pTmp->m_next != NULL)
			pTmp = pTmp->m_next;
		pTmp->m_next = new ListNode(e,NULL);
	}
}

here is my add to front
1
2
3
4
5
6
 
void List::Add_To_Front(const int &e)
{
	m_count++;
	ListNode* pTmp = new ListNode(e, m_first);
	m_first = pTmp;
}


i get no output. Does my code look good. mainly the copy constructor because the other adds work fine

Thanks
Last edited on
Disch (13742)
Your copy constructor doesn't quite copy everything. It's not copying the first node, it's just pointing to it. This will be a problem, as you'll end up deleting the same node multiple times.

You also don't initialize your variables (m_count is never initialized)

Try this:

1
2
3
4
5
6
7
8
9
10
11
12
13
 
List::List(const List& aList)
{
	m_first = NULL;  // initialize these
	m_count = 0;

//	m_first = aList.m_first;  // get rid of this
	ListNode* pTmp = m_first;
	for(int i = 0; i < aList.get_count(); i++)
	{
		Add_To_Back(pTmp->m_element);
		pTmp = pTmp->m_next;
	}
}
btripp (160)
Heres my header file if it helps.

I just made the changes you said and I get the following error

Unhandled exception at 0x00231ef9 in LinkedList.exe: 0xC0000005: Access violation reading location 0x00000000.

Are my steps right.

1. set m_first = NULL and count to 0 // basically an empty list for now
2. next I would think you have to make a ListNode* pTmp in order to traverse down aLIst, so go ahead and set that to aList.m_first instead of just m_first like above.
3. since i know how many nodes are on aList i would for loop; as long as i <aList.get_count()
4. for every iteration call the add_to_back(pTmp->m_element) which would get the element out of the current node pTmp, then simply update pTmp to the next node and do the same.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
 
#ifndef LIST_H
#define LIST_H

#include <iostream>
#include "ListNode.h"

class List
{
public:
	
	List();
	List(const List& aList);
	~List();
	
	int get_count()const;//returns m_count which is modified when list is made
	int count()const; //goes down list and counts

	bool Delete_From_Front(int& element);
	bool Delete_From_Back(int& element);
	void Add_To_Front(const int& e);
	void Add_To_Back(const int& e);

	void List_To_Array(int*& pTr, int& size); //copy elements of a list to a dynamic array
	void print()const;
	int Max()const;
	int Min()const;
	int Total()const;
	float Avg()const;

	List& operator += (const int& element); // adds a number to end of list


	//copy constructor
	//array to list

	// = operator
	// + operator list + list and list + element
	//sort
	//insert sort??
private:
	ListNode* m_first;
	int m_count;	
};
#endif
btripp (160)
sweet. It works now. I just had to change it to the following

1
2
3
4
5
6
7
8
9
10
11
12
 
List::List(const List& aList)
{
	
	m_first = NULL;
	m_count = 0;
	ListNode* pTmp = aList.m_first;
	for(int i = 0; i < aList.get_count(); i++)
	{
		Add_To_Back(pTmp->m_element);
		pTmp = pTmp->m_next;
	}
}


Disch (13742)
I'm sorry... I made a goof:

1
2
 
//  ListNode* pTmp = m_first;  // bad
    ListNode* pTmp = aList.m_first;  // good


Other than that the copy ctor looks good, provided Add_To_Front and Add_To_Back work properly (which I admittedly didn't look very closely at)

*looks*

Yeah they look fine.

Although you might want to consider adding a m_last member so you don't have to traverse the list every time you want to add/remove the last node.

EDIT: doh I'm too slow.

Glad you got it working.
Last edited on
btripp (160)
good idea. I will do that.

Thanks alot
Topic archived. No new replies allowed.