C++ NULL pointers and const correctness

Question

I read that it is good practice to do a check in the destructors of classes after deletion for pointer data members as follows:

if( 0 != m_pPointer)
{
    delete m_pPointer;
    m_pPointer= 0;
}

However, I found out that this prevents you to declare const pointers as data members as follows:

Type* const m_pPointer;

Isn't assigning NULL to pointers(as in my example above) a barrier for const-correctness? What is the best way to do? Keep everything const and stop assigning NULL to the deleted pointer or declaring non-const pointers even though their address never changes?

Answer 1

This is bad practice for the following reasons:

1. Setting a pointer to null in the destructor may mask double destruction problem. Good practise is to detect problems as early as possible.
2. Checking a pointer for null before deleteing it only adds unnecessary code. delete handles null pointers by doing nothing. Good practice is to minimize the amount of code.

Answer 2

Deleting a null pointer is guaranteed safe, so that null check is pointless.

If a class has a member that is a const pointer to a non-const object then you're saying the pointer value WILL NOT change within the lifetime of the wrapping object - that being the case you should only do this in the case where the object pointed to will live as long or longer than the wrapping object and the wrapping object will never want to point to a different object.

The fact that you have this issue simply means you've used a const pointer in the wrong place. You claim that in your case the pointer value never changes, but in your example it obviously does - it changes to null.

Answer 3

The "best way to do" is:

class foo {
  std::unique_ptr<bar> m_pPointer;
public:
  foo(std::unique_ptr<bar> pPointer)
    : m_pPointer{std::move(pPointer)} {}
};

or for const,

class foo {
  const std::unique_ptr<bar> m_pPointer;
public:
  foo(std::unique_ptr<bar> pPointer)
   : m_pPointer{std::move(pPointer)} {}
};

No new, no delete, no destructor.

Answer 4

A weird situation can be caused when you link a static lib with a global or static object from two different shared libs (on Linux) which later be linked to the same executable.

Each shared lib object insert call to constructor and destructor, so you'll have one object and two calls for constructor and destructor for the same object (actually you'll have 2 objects mapped to the same address).

You'll probably find the problem when your app crash in the 2nd destructor. if you NULL it you'll never know that there was a problem at all.

for your question: except for the above issue, I think you should distinct two types of pointers: See the class below:

class A{
  obj *x, *y;
  A(){
    x = new obj;
    y = NULL
  }
  ~A(){
    delete x;
    if(y)delete y; // the `if` here will save the calling and returning run time when NULL. 
  }
  void RecicleX(){
    delete x;
    x = new obj;
  }
  void InitY(){
    assert(y==NULL); //illegal to call init when already
    y = new obj;
  }
  void TermY(){
    assert(y); //illegal to call term when already inited
    delete y;
    y = NULL; //prevent crush in dtor if called after...
  } 
};

x is always exists, so no need to check it, and no need to null it. y may exists and may not, so I think you should null it after deletion. (You maybe will want also to know the current state, like for assert)