Doubly Linked List

kinderu

In the following code, I've implemented a doubly linked list in which each node is a structure with the following fields: firstName, secondName, CNP, Email; When I call the function ptr->DeleteNODE for the fifth time to delete the last node left in the list the code crashes. Deleting nodes is done from the beginning of the list. I don't understand why it's crashes.

#include #include using namespace std;

struct Persons
{
string firstName;
string secondName;
string CNP;
string Email;
};

class NODE
{
private:
NODE *next;
NODE *previous;
public:
Persons *box = new Persons();
NODE(string firstName, string secondName, string CNP, string Email)
{
box->firstName = firstName;
box->secondName = secondName;
box->CNP = CNP;
box->Email = Email;
}
void SetNext(NODE *next)
{
this->next = next;
}
NODE* GetNext()
{
return next;
}
void SetPrevious(NODE *previous)
{
this->previous = previous;
}
NODE *GetPrevious()
{
return previous;
}
};

class DoublyLinkedList
{
private:
NODE *head;
public:
DoublyLinkedList()
{
head = NULL;
}
bool isEmpty()
{
return head == NULL;
}
void AddNODE(NODE *newNode)
{
if (isEmpty())
{
newNode->SetPrevious(NULL);
head = newNode;
}
else
{
NODE *temp = head;
while (temp->GetNext() != NULL)
temp = temp->GetNext();
temp->SetNext(newNode);
newNode->SetPrevious(temp);
}
newNode->SetNext(NULL);
}
void DeleteNODE()
{
if (isEmpty())
cout << "\n List is Empty." << endl;
NODE *temp = head;
if (head->GetNext() != NULL)
{
head = head->GetNext();
head->SetPrevious(NULL);
}
else
head = NULL;
delete temp;
}
void Print()
{
NODE *temp = head;
while (temp != NULL)
{
cout << "\n First Name : " << temp->box->firstName;
cout << "\n Second Name : " << temp->box->secondName;
cout << "\n CNP : " << temp->box->CNP;
cout << "\n Email : " << temp->box->Email;
temp = temp->GetNext();
cout << endl;

Lost User

Use your debugger to find the point where it crashes and why. Also, when you create a node you use the same box* each time. I think you should create a new one in the constructor, or maybe move the persons properties into the node class.

leon de boer

You aren't cleaning up the pointer chain at all Get out a piece if paper and draw 5 box and draw arrows as next and prev Now there are 3 cases 1.) delete the first one and draw what needs to happen (it has a special pointer to it .. problem) 2.) delete the middle one and draw what needs to happen to the pointers. (2 pointers to clean up) 3.) delete the last one and draw what needs to happen (which may or may not differ to 2 depends how you set it up) Paper and planning are you friend in programming.

In vino veritas

leon de boer

You aren't cleaning up the pointer chain at all Get out a piece if paper and draw 5 box and draw arrows as next and prev Now there are 3 cases 1.) delete the first one and draw what needs to happen (it has a special pointer to it, you called it head .. problem) 2.) delete the middle one and draw what needs to happen to the pointers. (2 pointers to clean up) 3.) delete the last one and draw what needs to happen (which may or may not differ to 2 depends how you set it up) Paper and planning are you friend in programming.

In vino veritas

Jochen Arndt

Your NODE class does not initialise the next and previous members in the constructor. You are initialising them in your DoublyLinkedList class when adding a node. But so valid states depend on that which is dangerous and bad style. You should initialise them in the constructor to ensure that they contain always a defined value. You are allocating your objects on the stack but in DeleteNODE you call delete. Sou you should allocate them on the heap instead:

NODE *obj1 = new NODE("Dragu", "Stelian", "1911226284570", "dragu_stelian@yahoo.com");
ptr->AddNODE(obj1);

To avoid such wrong allocation, a linked list implementation usually does the allocation (e.g. by not passing an existing object to the add function but the data and let the implementation do the allocation):

void DoublyLinkedList::AddNODE(const Persons& person)
{
// Requires a constructor that accepts a Persons reference
NODE *newNode = new NODE(person);
// ...
}

There is also no need to use heap allocation for the box member (which is never getting deleted in your implementation). Just use the structure as member. Instead of finding the reason of your crash I suggest to rethink your design. There are plenty of examples.