Copy Constructor and Assignment Operator

Hey guys, I have been fighting with this for hours, but cannot seem to fix it.I am trying to make a copy constructor and an assignment operator use other classes one.

I have a circular dependency of a Stack, then implement a queue in terms of a stack, then a list in terms of a queue.

here is the error I am getting.

1
2
  List.cpp:17:10: error: comparison between distinct pointer types ‘List<int>*’ and ‘Queue<int>* const*’ lacks a cast [-fpermissive]
  if(this != &other.queue)
here is the List copy constructor.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17

template <class T>
List<T> :: List(const List<T>& other)
{
	queue =new Queue<T>(*other.queue);
}

template <class T>
List<T>& List<T> :: operator=(const List<T>& other)
{
	if(this != &other.queue)
	{
		queue =new Queue<T>(*other.queue);
	}
	return *this;
}


I tried dynamic binding but could not get it
Why are you comparing this against other.queue?

1
2
3
4
if (this != &other)
{
    // ...
}

I am not yet that good with c++.But I was taught that makes sure you dont copy itself
You probably want to compare against &other.
bookerb6 wrote:
But I was taught that makes sure you dont copy itself

Yes. You must prevent the copying if this and &other are the same.

this is a pointer to the current object (this means it holds the current object's memory address).
You check it against the other object's memory address.
You do not check it against the memory address of other.queue, which is a member variable of other.

Also, be warned that in the C++ library there exists a container named std::queue.
You should probably not use the name "queue" in your code, especially if you are using namespace std;.
Last edited on
Unfortunatelly I am given the .h files and have to implement the .cpp files.I also have a problem with the ostream operator.

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

template <class T>
ostream& operator<<(ostream& os,List<T>& list)
{
	Queue<T>* temp = new Queue<T>();
	Queue<T>* t = new Queue<T>();
	
	while(!list.queue->isEmpty())
	{
		temp->enqueue(list.queue->dequeue());
	}
	
	cout << "[";
	
	while(!temp->isEmpty())
	{
		T num = temp->dequeue();
		//cout << num <<endl;
		list.queue->enqueue(num); //seems like this line does not work
		os << num;
		
		
		if(!temp->isEmpty())
		{
			//list.queue->enqueue(num);
			cout << ",";
		}
	}
	
	cout << "]";

}


here I am attempting to print the results, but not deleting the queue.But it seems like list.queue(num); does not work.I am confused now guys.What am I really doing wrong
You are overusing dynamic memory allocation (pointers + new).
Then you have memory leaks because you don't deallocate the memory by delete'ing the pointers.

1
2
3
4
5
6
7
8
9
10
// instead of...
	Queue<T>* temp = new Queue<T>();
	Queue<T>* t = new Queue<T>();
	// ...
	delete temp;
	delete t;

// do this:
	Queue<T> temp;
	Queue<T> t;


Another thing: the output operation should not modify the object which is output.
In this case, the code should work even if we pass list by const reference.
(Also just as in the case of queue vs std::queue, there's list vs std::list, you've been warned. Twice.)

1
2
3
4
5
6
ostream& operator<<(ostream& os, const List<T>& list)
{
	// ...

	return os;
}


Why did you not implement a copy constructor (and copy assignment operator) for your queues?

Then you would not have to modify the list parameter, you could simply do:

Queue<T> temp = list.queue; // copy list's queue into temp

Edit: you are overusing dynamic memory allocation in your List class template as well.
Why is List::queue a pointer?
Last edited on
Thanks alot Catfish.
It seems to be working now.The reason I dont use the assignment operator is because for some reason the List one is not working well.

I am confused as to how can that occur.The stack and the queue one are working fine, but not the List one.

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
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86

// assignment operator
template <class T>
Stack<T>& Stack<T> :: operator=(const Stack<T>& other)
{
	if(this != &other)
	{
		Node  *temp, *curr, *last;
		
		if(top != NULL)
		{
			Node * temp;
			
			while(top != NULL)
			{
				temp = top;
				top = top->next;
				
				delete temp;
			}
			//intialiuze stack
		}
		if(other.top == NULL)
		{
			top = NULL;
		}
		else
		{
			
			      curr = other.top;
				top = new Node(other.top->element);// not sure
				top->element = curr->element;
				
				top->next = NULL;
				last = top;
				curr = curr->next;
				
			//	Node * temp;
						while(curr != NULL)
					{
						temp = new Node(curr->element);//not sure
						
						temp->element = curr->element;
						temp->next = NULL;
						last->next = temp;
						last = temp;
						curr = curr->next;
					}
		
		}
	}
	
	return *this;
	
}


here is my queue assignment operator


template <class T>
Queue<T>& Queue<T> :: operator=(const Queue<T>& other)
{
	if(this != &other)
	{
		stack =new Stack<T>((*other.stack));
	}
	return *this;
}


//and this is my List assignment operator


template <class T>
List<T>& List<T> :: operator=(const List<T>& other)
{
	if(this != &other)
	{
		queue =new Queue<T>((*other.queue));
	}
	return *this;
	
	
}



The Stack and the queue is fine, but the List is not.How is that possible, because it uses the other two.
Both Queue<T>::operator= and List<T>::operator= leak memory. In the case of the List twice as much memory is leaked as it also uses the Queue version. If there is anything wrong with List<T>::operator=, it is also wrong with Queue<T>::operator=.

Does it not seem a little perverse that Stack uses a linked list for it's implementation, and you're ultimately using a Stack for a linked list implementation? It would make a lot more sense for the List to stand alone, and to implement Queue in terms of the List, and Stack in terms of the Queue. You usually want to start with the general and proceed to the specific (or more restricted) which is the opposite of what you're doing here.

Topic archived. No new replies allowed.