Should I delete QPointer in the destructor of the class?

Question

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?

Answer 1

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.

Answer 2

QPointer is not a smart pointer. It does not manage the object it points to. It just tracks whether or not it was deleted. You will need to delete it yourself:

~MyClass()
{
    delete m_process.data();
}

When you delete the pointed-to object, data() will be null.

You don't need to check if it's null before deleting it, because deleting a null pointer is OK (it just doesn't do anything.)

I don't think you need a QPointer here though. As said before, this class is only useful for tracking whether or not the object has been deleted elsewhere. You should probably not be using a pointer at all here. Just do:

#include <QProcess>

class MyClass: public QObject {
private:
    QProcess m_process;
// ...

And change your connect code to:

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