//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;
}
}
Setting pointers to NULL in deconstructor
Author
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:
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?
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.
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.
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:
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;}delete [] m_pArray;
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.
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.
Quote:Original post by MadMax1992It'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!
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?
Yes something must never be destructed twice.
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
Quote:Original post by stonemetalQuote: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.
This topic is closed to new replies.
Advertisement
Popular Topics
Advertisement
