Search code examples
c++qtc++14qpointer

Should I delete QPointer in the destructor of the class?


Suppose I have a class like the following:

#include <Object>
#include <QProcess>

class MyClass: public QObject {
private:
    QPointer<QProcess> m_process{ nullptr };
public:
    MyClass(QObject *parent = nullptr)
        : QObject{ parent }
        , m_process{new QProcess{} }
    {
        QObject::connect(m_process, &QProcess::errorOccurred,
            this, [](QProcess::ProcessError error) {
            qDebug() << "error: " << error;
            });
    }
    ~MyClass()
    {
        if (m_process)  delete m_process; //--> is it necessary?
    }
};

Should I need to have the delete the m_process manually as shown in the destructor?

Unfortunately, I can not use std::unique_ptr or std::shared_ptr, as of

QObject::connect(m_process, &QProcess::errorOccurred,
            this, [](QProcess::ProcessError error) {
            qDebug() << "error: " << error;
            });

I have not seen a proper overload for QObject::connect.

On the other hand in QPointer::~QPointer() I have read:

Destroys the guarded pointer. Just like a normal pointer, destroying a guarded pointer does not destroy the object being pointed to.

That means QPointer::~QPointer() will delete as MyClass's object go out of scope, and hence I am deleting the m_process twice?

Or did I misunderstood?


Solution

  • Purpose of QPointer is to provide a guarded or weak pointer to a QObject subclass. It does not delete the object when it goes out of scope, it just knows if the object it points to is alive or deleted already.

    So your current code is correct in that sense. A few comments:

    • It's not useful to have default value nullptr for the pointer in variable declaration, because you initialized it in the constructor initializer list.
    • You don't need to check if a pointer is null before deleting, because delete nullptr; is valid code which just does nothing.
    • If the lifetime of your QProcess is same as the lifetime of the containing object, then you should just put it as a member variable, and not use new at all, unless you have some specific reason.
    • If you do want to use new to allocate it, consider if the QProcess could have a parent QObject, which will delete it.
    • Alternatively, you should wrap the pointer in QScopedPointer or std::unique_ptr, because they own the object they point to and will delete it when they go out of scope.