Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

My linked-list node removing function causes other parts of my program to crash

I'm encountering a weird error in some homework that has me making a stack with a linked list. All functions in my program work perfectly, even in the beginning when I have no nodes, or after deleting a few. But when I make nodes and delete them all so I'm back to zero, then every function causes a crash. I've tried researching the problem, but the solutions I've found look almost identical to what I've already got, so obviously there's something critical I'm not seeing.

Here's the node removal function (the one I suspect to be the culprit in all this)

void remove(node** root)
{   
    node* temp = *root;
    node* previous = 0;
    if(*root)
    {
        while((*root)->next)
        {
            previous = *root;
            *root = (*root)->next;
        }
        delete *root;
        *root = temp;
        if(previous)
        {
            previous->next = 0;
        }
    }
    else
    {
        std::cout<<"cannot delete items from empty list\n";
    }
}

Here's the node insert function

void insert(node** root)
{
    node* temp = *root;
    if(*root)
    {
        while((*root)->next)
        {
            (*root) = (*root)->next;
        }
        (*root)->next = new node;
        (*root)->next->data = getnum();
        (*root)->next->next = 0;
        *root = temp;
    }
    else
    {
        (*root) = new node;
        (*root)->data = getnum();
        (*root)->next = 0;
    }

}

I'm fairly sure that the issue is somewhere in the code I linked, but just in case it's not, here's a pastebin to the full assignment http://pastebin.com/AWtG4qjD

like image 958
kamstack Avatar asked Jul 20 '12 17:07

kamstack


1 Answers

remove implementation is not correct.Suppose the list has one element . In this case temp will point to "non-existing memory" after you execute delete *root; However what you are doing is *root = temp; this way you cause root to point to invalid node. And this causes the weird behavior later The possible way to make your implementation correct is :

void remove(node** root)
{
    //TODO: your code here
    node* temp = *root;
    node* previous = 0;
    if(*root)
    {
        while((*root)->next)
        {
            previous = *root;
            *root = (*root)->next;
        }
        delete *root;
        if(previous)
        {
            *root = temp;
            previous->next = 0;
        }
        else {
            *root = NULL;
        }
    }
    else
    {
        std::cout<<"cannot delete items from empty list\n";
    }
}

But I don`t advise you to iterate the list with root pointer .Define some iterator and look for the end with it instead of changing *root

like image 179
Yakov Avatar answered Oct 26 '22 22:10

Yakov