-2

I've been working on my destructor for a linked list project where even numbers are inserted in the front and odd numbers are inserted in the back. Deletion works on a last in first out depending on if it is odd or even. I have everything working but I keep getting an error when I run my destructor. I've walked through the debug and it deletes all of the nodes but then crashes afterwards and I'm not sure why. Here is my code:

//Headerfile
class Staque
{
private:
struct staqueNode
{
    int value;
    struct staqueNode *next;
    struct staqueNode *prev;
};

staqueNode *root;


public:
Staque();
~Staque();

void addNode(int addIn);
void deleteNode(int oddIn, int evenIn);
void display();
};

//header.cpp file
#include"Staque.h"
#include<iostream>

using namespace std;

Staque::Staque()
{
root = NULL;    
}

void Staque::addNode(int addIn)
{
staqueNode *newNode;
staqueNode *nodePtr;
staqueNode *temp = NULL;

newNode = new staqueNode;
newNode->value = addIn;
newNode->next = NULL;
newNode->prev = NULL;
if (!root)
{
    root = newNode;
}
else
{
    if (newNode->value % 2 == 0)
    {
        nodePtr = root;
        while (nodePtr->next)
        {
            nodePtr = nodePtr->next;
        } 
        nodePtr->next = newNode;
        newNode->prev = nodePtr;
    }
    else if (newNode->value % 2 != 0)
    {
        nodePtr = root;
        while (nodePtr->prev)
        {
            nodePtr = nodePtr->prev;
        }
        nodePtr->prev = newNode;
        newNode->next = nodePtr;
    }
}
} 
void Staque::deleteNode(int oddIn, int evenIn)
{
staqueNode *nodePtr;
staqueNode *temp = NULL;
if (!root)
    return;
while (evenIn > 0)
{
    nodePtr = root;
    while (nodePtr != NULL && nodePtr->next != NULL)
    {
        temp = nodePtr;
        nodePtr = nodePtr->next;
    }
    if (nodePtr == root && root->value % 2 == 0)
    {
        root = root->prev;
        temp->next = NULL;
        delete nodePtr;
        evenIn = 0;
    }
    else
    {
        temp->next = NULL;
        delete nodePtr;
        evenIn -= 1;
    }
}

while (oddIn > 0)
{
    nodePtr = root;
    while (nodePtr != NULL && nodePtr->prev != NULL)
    {
        temp = nodePtr;
        nodePtr = nodePtr->prev;
    }
    if (nodePtr == root && root->value % 2 != 0)
    {
        root = root->next;
        temp->prev = NULL;
        delete nodePtr;
        oddIn = 0;
    }
    else
    {
        temp->prev = NULL; 
        delete nodePtr;
        oddIn -= 1;
    }
}

}

void Staque::display()
{
staqueNode *nodePtr;

nodePtr = root;
while (nodePtr->next)
{   
    nodePtr = nodePtr->next;
}
cout << "\nThe staque: ";
while (nodePtr->prev)
{
    cout << nodePtr->value << " ";
    nodePtr = nodePtr->prev;
}
cout << nodePtr->value << endl;

}

Staque::~Staque()
{
staqueNode *nodePtr;
staqueNode *temp;
nodePtr = root;

while (nodePtr->next)
{
    temp = nodePtr;
    nodePtr = nodePtr->next;
}
//nodePtr = root;
while (nodePtr->prev)
{
    temp = nodePtr;
    nodePtr = nodePtr->prev;
    delete temp;
}
//delete root;

}


//source/main.cpp
#include"Staque.h"
#include<iostream>


using namespace std;

int main()
{
Staque myList;
int choice;
int input;
int numOdd;
int numEven;

do
{
    cout << "Would you like to: \nAdd a node: 1\nDelete a node: 2\nDisplay the list: 3\nQuit: 0\n";
    cin >> choice;
    while (choice < 0 || choice > 3)
    {
        cout << "Invalid Input: Would you like to: \nAdd a node: 1\nDelete a node: 2\nDisplay the list: 3\nQuit: 0\n";
        cin >> choice;
    }
    switch (choice)
    { 
    case 1:
        cout << "Enter the value you would like to add to the list: ";
        cin >> input;
        while (isalpha(input))
        {
            cout << "Invalid input: Enter the value you would like to add to the list: ";
            cin >> input;
        }
        myList.addNode(input);
        myList.display();
        break;

    case 2:
        cout << "Enter the number of even numbers you would like to delete: ";
        cin >> numEven;
        while (isalpha(input) || numEven < 0)
        {
            cout << "Invalid input: Enter the number of even numbers you would like to delete: ";
            cin >> numEven;
        }

        cout << "Enter the number of odd numbers you would like to delete: ";
        cin >> numOdd;
        while (isalpha(input) || numEven < 0)
        {
            cout << "Invalid input: Enter the number of odd numbers you would like to delete: ";
            cin >> numOdd;
        }
        myList.deleteNode(numOdd, numEven);
        myList.display();
        break;

    case 3:
        myList.display();
        break;

    default:
        break;

    }
} while (choice != 0);

myList.~Staque();

return 0;

}
Brian Tompsett - 汤莱恩
  • 5,753
  • 72
  • 57
  • 129
JBoyden
  • 13
  • 4
  • `I have everything working but I keep getting an error when I run my destructor` Which could conceivably mean that a whole lot is *not* working. Also, please fix your formatting. – PaulMcKenzie Oct 20 '14 at 21:17
  • 1
    I suspect you didn't obey the [Rule of Three](http://stackoverflow.com/q/4172722/10077). – Fred Larson Oct 20 '14 at 21:18
  • 3
    `myList.~Staque();` ??!! Why are you doing this? Remove it. – PaulMcKenzie Oct 20 '14 at 21:20
  • @PaulMcKenzie: Good catch. That could account for it! – Fred Larson Oct 20 '14 at 21:22
  • _@JBoyden_ You actually have **bad** code! Debug, step through. Debug, have break points. Debug, trace back stack frames that initiated the exception. – πάντα ῥεῖ Oct 20 '14 at 21:23
  • and if for some reason you want to keep your object in a valid state after destruction you should probably set `root = NULL;` at the end of the destructor. (just using `NULL` because you did, normally I would use `nullptr`) – PeterT Oct 20 '14 at 21:28

2 Answers2

1

One obvious error is that you're calling your destructor explicitly.

myList.~Staque();

You shouldn't do this -- the object will "die" naturarlly when it goes out of scope. By calling the destructor explicitly, the object will have the destructor called, then the destructor will be called a second time when the object goes out of scope. It is the second invocation that will cause all havoc to break loose.

The time when you should call the destructor explicitly is if you're using placement-new, and you're not doing that here. Therefore just remove that line above and see if the error goes away. If it doesn't, then you have further problems, but at the very least, you would remove this glaring issue from your code.

PaulMcKenzie
  • 34,698
  • 4
  • 24
  • 45
  • Thank you, That fixed the error I was getting. I thought I had to call explicitly call it or it would leave the memory in use. – JBoyden Oct 20 '14 at 21:52
0

Looking at your destructor specifically...

What happens when nodePtr is null? Eg you didn't create a node.

Or you get to the last node which will always set the next ptr to NULL and you then dereference nodePtr->next on a null pointer - yes you guess it a crash.

in your while (nodePtr->next) what is the temp assignment doing? Nothing useful it seems.

while (nodePtr->prev) ???

No check of nodePtr being null?

Thats something to start with.

Are you even sure the rest of it works?

Angus Comber
  • 9,316
  • 14
  • 59
  • 107