3
\$\begingroup\$

I'm a novice regarding both C++ and Qt. This class is used in my project to get an image from a web service.

If a new call to StartDownload is made before the previous has finished, the old one can safely be discarded as it won't be needed any more. This is because a new request is made for each resize event, as one of the parameters to the service is the image size (font size in the image is adapted to the image size).

As a result, when a request is canceled, Qt prints an error message: QCoreApplication::postEvent: Unexpected null receiver. This does not interfere with any functionality, but tells me I haven't handled canceling the request properly. Several of these messages are printed as I resize the window and CancelDownload is called.

Aside from that, any general feedback on my design or C++ idiomacy is welcome!

Note: I'm using C++11 and Qt 5.6.

Header

#ifndef DOWNLOADER_H
#define DOWNLOADER_H

#include <QtNetwork/qnetworkaccessmanager.h>
#include <qbytearray.h>
#include <QObject>

class Downloader: public QObject
{
    Q_OBJECT

public:
    Downloader();
    ~Downloader();
    void StartDownload(QUrl url);

private:
    void CancelDownload();

    QNetworkAccessManager mNetworkAccessManager;
    QNetworkReply *mCurrentReply;

signals:
    void DownloadFinished(QByteArray bytes);

private slots:
    void HandleFinishedDownload();
};

#endif // DOWNLOADER_H

Source

#include <QtNetwork/qnetworkrequest.h>
#include <QtNetwork/qnetworkreply.h>
#include "Downloader.h"

Downloader::Downloader()
    : mNetworkAccessManager(), mCurrentReply(Q_NULLPTR)
{
}

Downloader::~Downloader()
{
    CancelDownload();
    mNetworkAccessManager.deleteLater();
}

void Downloader::CancelDownload()
{
    if (mCurrentReply != Q_NULLPTR) {
        mCurrentReply->abort();
        mCurrentReply->deleteLater();
        mCurrentReply = Q_NULLPTR;
    }
}

void Downloader::StartDownload(QUrl url)
{
    CancelDownload();

    QNetworkRequest request(url);
    mCurrentReply = mNetworkAccessManager.get(request);
    connect(mCurrentReply, &QNetworkReply::finished, this, &Downloader::HandleFinishedDownload);
}

void Downloader::HandleFinishedDownload()
{
    if (!mCurrentReply) {
        qCritical("Null reply in HandleFinishedDownload");
        return;
    }

    if (mCurrentReply->error() == QNetworkReply::NoError) {
        emit DownloadFinished(mCurrentReply->readAll());
    }
    else if (mCurrentReply->error() != QNetworkReply::OperationCanceledError) {
        qWarning() << "Failed download:" << mCurrentReply->errorString();
    }

    mCurrentReply->deleteLater();
    mCurrentReply = Q_NULLPTR;
}

Update

Although I solved the issue described above in a self-answer, I am still looking for a general review!

\$\endgroup\$
1
\$\begingroup\$

The error message is caused by me misunderstanding the purpose of deleteLater. As implied by the documentation, the member function is only required when deleting the object inside a slot, like HandleFinishedDownload.

Note: Do not delete the object in the slot connected to the error() or finished() signal. Use deleteLater().

In all other cases, the standard delete keyword should be used. As a result, here is an updated implementation of CancelDownload.

void Downloader::CancelDownload()
{
    if (mCurrentReply != Q_NULLPTR) {
        mCurrentReply->abort();
        delete mCurrentReply; // this is the only altered line
        mCurrentReply = Q_NULLPTR;
    }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.