is Recursion ok in a deconstructor?

I was wonder if any problems might come up from using recursion in a properly coded deconstructor for a single linked list.

Code:

class MyList
{
ListData* pListData;
MyList* pNextNode;
}

MyList::~MyList()
{
	if (pListData)
		delete pListData;
	if (pNextNode)
		delete pNextNode;
}

ListData has it's own deconstructor that deletes whatever it needs to. Testing it with a cout seems to show it being called the correct number of times. Also, should I set pNextNode to NULL inside the deconstructor?

I see no problem with that and have done so myself. Also as long as you aren't dealing with a idiotic compiler the if statements are not needed. As for setting it to NULL that is more personal test then anything.

Originally Posted by Thantos

I see no problem with that and have done so myself. Also as long as you aren't dealing with a idiotic compiler the if statements are not needed. As for setting it to NULL that is more personal test then anything.

Yea, the if statements aren't needed as it is safe to delete a null pointer, however because the code provided doesn't set anything to null after deletion it does run the risk of calling delete a second time on a pointer that has already been deleted which is not so safe.

Ok so I should set the pointers to NULL to be safe. Is there a way to set the first pointer that started the recursion to NULL inside the deconstructor? this = NULL; doesn't work

Originally Posted by Darryl

Yea, the if statements aren't needed as it is safe to delete a null pointer, however because the code provided doesn't set anything to null after deletion it does run the risk of calling delete a second time on a pointer that has already been deleted which is not so safe.

That only matters if there are additional references to the pointer.

If the pointers belong are members of a particular object the destructor need not set the pointers to NULL as it is guaranteed that the pointers will not be deleted twice.

For example;

Code:

class X
{
     public:
        X () : data(new int) {};
        ~X() {delete data;};
        void Set(int a) {*data = a;};
        void DoNothing() {};
     private:
        int *data;
};
 int main()
{
     X *x = new X;

     delete x;

     //   it is impossible (without invoking undefined behaviour) to access the old x->data     

     x->Set(5);             // one example of undefined behaviour
     x->DoNothing();   // and another
}

Both of the calls x->Set(5) and x->DoNothing() invoke undefined behaviour because x is now a dangling reference. The technique of setting x to NULL after deleting it in main(), and then checking it before invoking member functions, will work. However setting data to NULL inside X's destructor does not change the fact that main() invokes undefined behaviour.

The only exception to the above is if the object x gives some other object the ability to hold a pointer to it's data member. But, even then, changing data to NULL in the destructor does not fix that unless the destructor somehow communicates with the object holding the dangling reference to data.

Incidentally, "destructor" is the more usual name for X~X(), not "deconstructor".

Originally Posted by Syneris

Ok so I should set the pointers to NULL to be safe. Is there a way to set the first pointer that started the recursion to NULL inside the deconstructor? this = NULL; doesn't work

the recursion would have been started by a call to the MyList destructor outside of this linked list.. so the answer to your question is no.
ex..

Code:

int main()
{
    MyList *foo = new MyList;
    // ... add/delete nodes and whatnot in here..
    delete foo;      <-- this is where you start the recursion from
    foo = 0;          <-- do this instead of _this=null
    return 0;
}

Ok so I should set the pointers to NULL to be safe.

"Real programmers" don't use NULL. You can set your pointers to 0 directly, which is what NULL does.

...and it's called a d-e-s-t-r-u-c-t-o-r.

grumpy already corrected me on destructor, and http://faq.cprogramming.com/cgi-bin/...&id=1043284376 say NULL is to be used for pointers only. Why even have NULL if it shouldn't be used?

C legacy. In C, NULL might or might not be a pointer with a numeric value of 0. On most systems, NULL was defined as (void*)0 (i.e. the value 0 cast to a void pointer), which could then implicitely be cast to any other pointer type. However, there might be systems where this isn't so.
C++, however, doesn't allow the implicit cast from void* to non-void*. The definition as appears in most stdlib.h files doesn't work. Stroustrup made a rule in C++, though, which says that the literal 0 can be implicitely cast to any pointer type, and the result of the cast will be an appropriate null pointer for that platform.
Therefore, you can safely use 0 instead of NULL in C++, which wasn't possible in C. Of course, some people think using NULL increases readability, because it's clear you're setting a pointer.
The NULL definition in C++ is just 0, so stdlib.h usually looks like this:

Code:

#ifdef __cplusplus /* If we're compiling as C++. */
#  define NULL 0
#else /* C */
#  define NULL ((void*)0)
#endif

since you mentioned Stroustrup..
here is what he says about using NULL or 0..

i agree that 0 not NULL is the c++ way, even though it seems undefined what the *c++ way* is in this case..

Using 0 makes code readability suffer and if they do the same thing why not make it easier to read.

Code:

int x=0;

That's a value.

Code:

m_pObject=NULL;

That's a pointer.

If you stay with that convention you can spot pointers a mile away. I see nothing wrong with it and think the whole argument over it is rather absurd.

>> I see nothing wrong with it and think the whole argument over it is rather absurd.

as are many a conversation in the comp sci world..

but atleast these arguments give us somthing to do other than be productive..

I tend to avoid raw pointers anyway, and all smart pointers initialize themselves to null for me.

m_pObject=NULL;

If you stay with that convention you can spot pointers a mile away.

Some might recognize that as a pointer before ever getting to the NULL part.

Yeah if you work for Microsoft.