Avoiding null pointer crashes in C++ by overloading operators - bad practice?

Question

I'm starting to write a rather large Qt application and instead of using raw pointers I want to use smart pointers, as well as Qt's own guarded pointer called QPointer.

With both standard library smart pointers and Qt's pointers the application crashes when a NULL pointer is dereferenced.

My idea was that I could add a custom overload to the dereference operators * and -> of these pointer types that check if the pointer is NULL.

Below is a short example that works fine so far. If a NULL pointer was dereferenced, a temporary dummy object would be created so that the application does not crash. How this dummy object would be processed might not be always correct, but at least there would be no crash and I could even react on this and show a warning or write it to a log file.

template <class T>
class Ptr : public std::shared_ptr<T> {

private:
  T * m_temp;

public:
  Ptr<T>(T * ptr) : std::shared_ptr<T>(ptr), m_temp(NULL) {}
  ~Ptr() { 
    if (m_temp) {
      delete m_temp;
    }
  }
  T * operator->() {
    if (!std::shared_ptr<T>::get()) {
      if (m_temp) {
        delete m_temp;
      }
      m_temp = new T();
      return m_temp;
    } else {
      return std::shared_ptr<T>::get();
    }
  }
  T & operator*() {
    return *operator->();
  }
};

Of course I'll be doing NULL checks and try to eliminate the source of NULL pointers as much as possible, but for the rare case that it I forget a NULL check and the exception occurs, could this be a good way of handling it? Or is this a bad idea?

Answer 1

I would say this is a bad idea for a few reasons:

1. You cannot derive from standard library types. It may work until you change something benign in your code and then it breaks. There are various things you can do to make this more acceptable, but the easiest thing is to just not do this.
2. There are more ways to create a shared_ptr than just a constructor call. Duplicating the pointer value in your m_temp variable is likely just to lead things to be out of sync and cause more problems. By the time you cover all the bases, you will have probably re-implemented the whole shared_ptr class.
3. m_temp = new T(); seems like a frankly crazy thing to do if the old pointer is null. What about all the state stored in the object that was previously null? What about constructor parameters? Any initialization for the pointer? Sure, you could maybe handle all of these, but by that point you might as well handle the nullptr check elsewhere where things will be clearer.
4. You don't want to hide values being nullptr. If you have code using a pointer, it should care about the value of that pointer. If it is null and that is unexpected, then something further up the chain likely went wrong and you should be handling that appropriately (exceptions, error codes, logging, etc.). Silently allocating a new pointer will just hide the original source of the error. Whenever there is something wrong in a program, you want to stop or address the problem as close to the source as possible - it makes debugging the problem simpler.

A side note, if you are confident that your pointers are not null and don't want to have to deal with nullptr in a block of code, you may be able to use references instead. For example:

void fun1(MyObject* obj) {}
void fun2(MyObject& obj) {}

In fun1, the code might need to check for nullptr to be well written. In fun2, there is no need to check for nullptr because if someone converts a nullptr to a reference they have already broken the rules. fun2 pushes any responsibility for checking the pointer value higher up the stack. This can be good in some cases (just don't try and store the reference for later). Note that you can use operator * on a shared_ptr/unique_ptr to get a reference directly.