2

I have created a class which includes a linked list and this class needs to have a destructor for deleting the linked list, which I have included however because the delete method itself calls the destructor I end up in an infinite recursive call situaction.

Here is the code:-

PolynomialNode::~PolynomialNode()
{
    /*PolynomialNode* current_node_ptr = link;
    PolynomialNode* header_ptr = link;
    int index = 0;
    if( link != NULL)
    {
        while( current_node_ptr != NULL)
        {
            index++;
            current_node_ptr = current_node_ptr->get_link();

        }

        delete_nodes( &header_ptr, 0, index);
    } */
    PolynomialNode* current_node_ptr = link;
    PolynomialNode* copy_ptr;
    while( current_node_ptr != NULL)

    {
        copy_ptr = current_node_ptr->get_link();
        current_node_ptr->set_link(NULL);
        delete current_node_ptr;
        current_node_ptr = copy_ptr;
    }
}

Note I tried this with a recursive call - intentional for deleting the linked list and I still have the same problem.

Any help would be much appreciated.

NB: I know it's a recursive call because when I step through the debugger I can see it happening.

hairyyak
  • 31
  • 6
  • related: http://stackoverflow.com/questions/1861912/should-delete-this-be-called-from-within-a-member-method – jldupont Jan 18 '10 at 16:05
  • I don't see how this will give infinite recursion - as long `set_link` does what it says, each recursive destructor call should return without recursing any further. – Mike Seymour Jan 18 '10 at 16:13
  • The resursive call happens when you try to delete a pointer to a linked list i.e. after the delete command - no I didn't think this would end up in a recursive either but when I use the debugger you can clearly see it happening. – hairyyak Jan 18 '10 at 16:49
  • Also because I have set the current_node_ptr to link there is no delete this. – hairyyak Jan 18 '10 at 16:49

3 Answers3

11

assuming link is the pointer to the next PolynomialNode, simply

PolynomialNode::~PolynomialNode()
{
    delete link;
}

will do the right thing recursively.

However, a better solution might be to retain your while() loop but move it out of the node to the destructor of a separate class representing the linked list as a whole.

moonshadow
  • 86,889
  • 7
  • 82
  • 122
  • 1
    Or even use std::list (or std::vector, since std::list is rarely a better option)... – Adam Bowen Jan 18 '10 at 16:08
  • 1
    If it's a cyclic list, you'll make a spectacular blowup. Also, if it's more nodes than the compilers max recursive depth you'll have another spectacular blowup :P – Kornel Kisielewicz Jan 18 '10 at 16:11
  • No it's not a cyclic list and I think it should be ok recursive wise because it's only 3/4 nodes at max. – hairyyak Jan 18 '10 at 16:16
  • 1
    The only problem with using std::list or std::vector is that I am attempting to do some exercises on C++ from a book - and the section is on linked lists - plus the book specifies I should use linked lists i.e. ones I've created myself. – hairyyak Jan 18 '10 at 16:20
  • That's is definatley going to fail if the loop is cyclic. Which the original question seems to suggest with his code. – Martin York Jan 19 '10 at 19:12
2

Add this line in the while loop, before delete:

if ( current_node_ptr == this ) continue;

This will allow you to avoid recursion, by skipping destruction of the node that is calling. However, you should ask yourself why the heck did this happen in the first place. I cannot answer that question, because I don't see the rest of the code.

Note: this is wrong unless you have a circular link, in case of which this would be a valid design, as long as you skip deleting this.

Kornel Kisielewicz
  • 55,802
  • 15
  • 111
  • 149
1

Assuming link is the pointer to the next PolynomialNode, simply

if(this->link != NULL) delete this->link;
erick2red
  • 1,312
  • 1
  • 14
  • 19