Sign in to follow this  

Setting pointers to NULL in deconstructor

This topic is 2814 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

The subject allready gives it away, I've been using code like this for a very long time now, because a few years ago, I learned it was 'good practice'. Most of my destructors of classes with dynamically allocated memory (or directX variables) look like this:
//example 1:
Class::~Class()
{
    if (m_pVar) {
        delete m_pVar;
        m_pVar = NULL;
    }
}

//example 2:
Class::~Class()
{
    if (m_pArray) {
        delete [] m_pArray;
        m_pArray = NULL;
    }
}

//example 3:
Class::~Class()
{
    if (m_pDirectXVariable) {
        m_pDirectXVariable->Release();
        m_pDirectXVariable = NULL;
    }
}
But why is this 'good practice', I mean: When the desctructor of a class is getting called, it can't be possible deleted again, right? Of course I'm talking about non-inherited classes, I can understand why this is useful for inherited classes, but it makes no sense for non-inherited classes. Is it (in non-inherited classes) just a waste of (the few) CPU cycles and program size?

Share this post


Link to post
Share on other sites
It's good practice to set dangling pointers to NULL, in general (not specifically in destructors).

As you point out, there's no real point doing it in a destructor, because the pointer (member variable) is being discarded after the destructor anyway...

Even in inherited classes it's not useful, unless your base class has public/protected members that you're deleting, which should be a pretty rare occurrence.

Share this post


Link to post
Share on other sites
I can't think of a reason to set the pointer to null if it can't be accessed after the body of the destructor has executed - that just seems like noise to me.

Also, it's fine to invoke delete or delete [] on a null pointer, so really, this:
if (m_pArray) {
delete [] m_pArray;
m_pArray = NULL;
}
Can be reduced to this:
delete [] m_pArray;
Of course manual deletion can often be avoided entirely by using an appropriate RAII container.

Share this post


Link to post
Share on other sites
The compiler doesn't care.

Depending on who you ask, a person may or may not care.

The code is functionally identical with or without the null. As long as that code doesn't change, it doesn't matter.

The problem is code tends to change all the time. At some point, it's very possible someone else might touch that destructor. And if they aren't paying attention, they may try to use the pointer again.

Of course, it's their own fault, but if setting the pointer to NULL can save them 20+ minutes of trying to track down a weird memory error (and referencing free'd memory is always weird), then the extra line is worth it.

But how often will that happen?

It's an extremely low-cost solution to a highly unlikely scenario. So you'll find people on both sides of the fence. The reality is that the extra line won't affect your code in the slightest, so whatever you choose, don't dwell on it. Get to work on something important.

Share this post


Link to post
Share on other sites
Quote:
Original post by MadMax1992
But why is this 'good practice', I mean: When the desctructor of a class is getting called, it can't be possible deleted again, right?
It's not good practice, it's pretty much only done by someone who takes some general advice given to them as a hard and fast rule, rather than engaging their brain to see when it is useful. Well done for actually questioning this rather than blindly accepting that it was of some bennefit!

Yes something must never be destructed twice.

Share this post


Link to post
Share on other sites
Quote:
Original post by MadMax1992
But why is this 'good practice', I mean: When the desctructor of a class is getting called, it can't be possible deleted again, right?


Does the class have a copy constructor? If not then it may get deleted twice(or more).
Quote:
Is it (in non-inherited classes) just a waste of (the few) CPU cycles and program size?

If the compiler can tell it isn't accessed again it will be optimized out so there should be no difference

Share this post


Link to post
Share on other sites
Quote:
Original post by stonemetal
Quote:
Original post by MadMax1992
But why is this 'good practice', I mean: When the desctructor of a class is getting called, it can't be possible deleted again, right?


Does the class have a copy constructor? If not then it may get deleted twice(or more).


But then the bug is the absence of the copy constructor.

Even in the case where you want to "share" the pointed-at thing between copied instances, it is still wrong to put this kind of logic in a destructor, because you want to clean up the pointed-at thing when the last copy reaches the destructor, and this approach will clean it up the first time instead. The only way around that is to have some kind of reference count, which typically means using a smart pointer.

Of course, you should normally just use some kind of smart pointer anyway, so that you don't have to worry about this destructor logic.

Share this post


Link to post
Share on other sites

This topic is 2814 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this