Trying to build an assignment Operator for a single linked list class. I thought I built it correctly, but am still getting a memory leak.
The class consists of a a First and Last variable. And then a Node structure.
The Node structure looks like this:
struct node
{
TYPE value;
node * next;
node * last;
};
My Assignment Operator looks like this, it still has a memory leak
queue& queue::operator=(const queue &rhs){
if(this == &rhs ){
node *ptr = first;
node *temp;
while(ptr != NULL){
temp = ptr;
ptr = ptr->next;
delete temp; // release the memory pointed to by temp
}
delete ptr;
} else{
if (rhs.first != NULL ) {
first = new node(*rhs.first);
first->next = NULL;
last = first;
// first-> value = v.first->value; // copy constructor should have done that
node *curr = first;
node *otherCur = rhs.first;
node *ptr = first;
node *temp;
while(ptr != NULL){
temp = ptr;
ptr = ptr->next;
delete temp; // release the memory pointed to by temp
}
while(otherCur->next != NULL){
curr->next = new node(*otherCur->next);
curr->next->next = NULL;
last = curr->next;
// curr->next->value = otherCur->next->value;
curr = curr->next;
otherCur = otherCur->next;
}
// curr->next = NULL;
}
}
return *this;
}
EDIT:
Copy Constructor(working):
// copy constructor
queue::queue(const queue &v){
if (v.first != NULL ) {
first = new node(*v.first);
first->next = NULL;
last = first;
// first-> value = v.first->value; // copy constructor should have done that
node *curr = first;
node *otherCur = v.first;
while(otherCur->next != NULL){
curr->next = new node(*otherCur->next);
curr->next->next = NULL;
last = curr->next;
// curr->next->value = otherCur->next->value;
curr = curr->next;
otherCur = otherCur->next;
}
// curr->next = NULL;
}
}
Working Destructor:
queue::~queue(){
node *ptr = first;
node *temp;
while(ptr != NULL){
temp = ptr;
ptr = ptr->next;
delete temp; // release the memory pointed to by temp
}
}
Member variables of the .H file:
private:
// fill in here
node * first;
node * last;
Instead of all of that code, if you have a working copy constructor and destructor, the assignment operator can be implemented easily using
copy / swap
.Basically all that's done is a temporary copy is made through using the copy constructor. Then the member(s) of
this
are swapped out with the temporary's members. Then at the end, the temporary will be deallocated (the destructor), taking along with it the old data.I know the code is tiny compared to your attempt, but it solves all the problems pointed out by others in the comments, adds exception safety, etc. and best of all, it works.
But remember, you must have a working, non-buggy copy constructor and destructor (in addition, your copy constructor must not use the assignment operator, which unfortunately many unaware programmers resort in doing). In addition, you must swap all the member variables, so if you add more member variables to your
queue
class, you need to add aswap
for each of those new variables.See this for information on the copy / swap idiom.