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!

asked Mar 23, 2016 at 22:41
\$\endgroup\$

1 Answer 1

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;
 }
}
answered Mar 29, 2016 at 21:12
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.