problem in using QHttp

[Ferdous]

I've written a class that will handle file downloads. I think my logic is ok but the program isn't working as expected. The program downloads the first file successfully but hangs with the second download. I've gone through my written code over and over again but can't find the logical error.

I've maintained one QHttp and one QFile object in the class and scheduled one GET request at a time. The idea has been taken from the book - C++ GUi programming with Qt.

Please help me in finding out the logical error.

myhttp.h

#ifndef MYHTTP_H
#define MYHTTP_H
 
#include <QObject>
 
class QHttp;
class QFile;
 
class MyHttp : public QObject
{
	Q_OBJECT
 
public:
	MyHttp(QObject* parent = 0);
	void get(const QString& strFullFilePath);
 
signals:
	void done();
 
private slots:
		void myHttpRequestDone(bool error);
 
private:
	void processNextRequest();
 
	QHttp* http;
	QFile* file;
	QStringList* pendingRequestList;
};
 
#endif

#ifndef MYHTTP_H
#define MYHTTP_H

#include <QObject>

class QHttp;
class QFile;

class MyHttp : public QObject
{
	Q_OBJECT

public:
	MyHttp(QObject* parent = 0);
	void get(const QString& strFullFilePath);

signals:
	void done();

private slots:
		void myHttpRequestDone(bool error);

private:
	void processNextRequest();

	QHttp* http;
	QFile* file;
	QStringList* pendingRequestList;
};

#endif

To copy to clipboard, switch view to plain text mode 

myhttp.cpp :

#include <QtNetwork>
 
#include "myhttp.h"
 
MyHttp::MyHttp(QObject* parent /* = 0 */) : QObject(parent)
{
	http = new QHttp;
	file = 0;
	pendingRequestList = new QStringList;
 
	connect(http, SIGNAL(done(bool)), this, SLOT(myHttpRequestDone(bool)));
}
 
void MyHttp::get(const QString& strFullFilePath)
{
	// Add this request to list
	pendingRequestList->append(strFullFilePath);
 
	if (pendingRequestList->size() == 1)
	{
		processNextRequest();
	}
}
 
void MyHttp::processNextRequest()
{
	if (pendingRequestList->isEmpty() == true)
	{
		emit done();
	}
	else
	{
		// Get the first request in the list
		QString strFullFilePath = pendingRequestList->first();
 
		// Extract the file name
		QUrl url(strFullFilePath);
		QFileInfo fileInfo(url.path());
		QString fileName = fileInfo.fileName();
 
		// Create an instance of a file with the extracted name and open it
		file = new QFile(fileName);
		file->open(QIODevice::WriteOnly);
 
		// Send the request
		http->setHost(url.host(), url.port(80));
		http->get(url.path(), file);
	}
}
 
void MyHttp::myHttpRequestDone(bool error)
{
	file->close();
 
	if (error == true)
	{
		file->remove();
	}
 
	delete file;
	file = 0;
 
	pendingRequestList->removeFirst();
	processNextRequest();
}

#include <QtNetwork>

#include "myhttp.h"

MyHttp::MyHttp(QObject* parent /* = 0 */) : QObject(parent)
{
	http = new QHttp;
	file = 0;
	pendingRequestList = new QStringList;

	connect(http, SIGNAL(done(bool)), this, SLOT(myHttpRequestDone(bool)));
}

void MyHttp::get(const QString& strFullFilePath)
{
	// Add this request to list
	pendingRequestList->append(strFullFilePath);

	if (pendingRequestList->size() == 1)
	{
		processNextRequest();
	}
}

void MyHttp::processNextRequest()
{
	if (pendingRequestList->isEmpty() == true)
	{
		emit done();
	}
	else
	{
		// Get the first request in the list
		QString strFullFilePath = pendingRequestList->first();

		// Extract the file name
		QUrl url(strFullFilePath);
		QFileInfo fileInfo(url.path());
		QString fileName = fileInfo.fileName();

		// Create an instance of a file with the extracted name and open it
		file = new QFile(fileName);
		file->open(QIODevice::WriteOnly);

		// Send the request
		http->setHost(url.host(), url.port(80));
		http->get(url.path(), file);
	}
}

void MyHttp::myHttpRequestDone(bool error)
{
	file->close();

	if (error == true)
	{
		file->remove();
	}

	delete file;
	file = 0;

	pendingRequestList->removeFirst();
	processNextRequest();
}

To copy to clipboard, switch view to plain text mode 

main.cpp :

#include <QCoreApplication>
 
#include "myhttp.h"
 
int main(int argc, char *argv[])
{
	QCoreApplication app(argc, argv);
 
	MyHttp* myhttp = new MyHttp();
 	QObject::connect(myhttp, SIGNAL(done()), &app, SLOT(quit()));
 
	myhttp->get("http://thedailystar.net/images/tdsmainlogo.jpg");
	myhttp->get("http://thedailystar.net/photo/2008/09/06/2008-09-06__front02.jpg");
	myhttp->get("http://ahmadferdous.googlepages.com/Ferdous.JPG");
 
	return app.exec();
}

#include <QCoreApplication>

#include "myhttp.h"

int main(int argc, char *argv[])
{
	QCoreApplication app(argc, argv);

	MyHttp* myhttp = new MyHttp();
 	QObject::connect(myhttp, SIGNAL(done()), &app, SLOT(quit()));

	myhttp->get("http://thedailystar.net/images/tdsmainlogo.jpg");
	myhttp->get("http://thedailystar.net/photo/2008/09/06/2008-09-06__front02.jpg");
	myhttp->get("http://ahmadferdous.googlepages.com/Ferdous.JPG");

	return app.exec();
}

To copy to clipboard, switch view to plain text mode 

[wysota]

QHttp::get() is an asynchronous operation, thus all of your myHttp::get calls are executed before the first download even starts. Thus the condition "pendingRequestList->size() == 1" is only satisfied right after you add the first request to the queue. You should queue requests and once a request is finished, check the queue and start another request.

[Ferdous]

Thanks for your reply.

You should queue requests and once a request is finished, check the queue and start another request.

That's what I've done here. I've queued all the download requests in pendingRequestList and scheduled only one download at a time. When the first download is finished and the request queue is empty, done(bool) signal is emitted and slot myHttpRequestDone(bool) is called where processNextRequest() is called and it's processNextRequest() where QHttp:get() is called for the next download. So it's not a problem that the condition pendingRequest.size() == 1 is executed only for the first download request.

Here I've emulated QHttp in blocking approach.

Please enlighten me further.

[wysota]

That's what I've done here.

Not really.

I've queued all the download requests in pendingRequestList and scheduled only one download at a time.

And that's exactly what happens - you get one download.

When the first download is finished and the request queue is empty, done(bool) signal is emitted and slot myHttpRequestDone(bool) is called where processNextRequest() is called and it's processNextRequest() where QHttp:get() is called for the next download. So it's not a problem that the condition pendingRequest.size() == 1 is executed only for the first download request.

Yeah, but after the first download you have two elements in the queue, so the condition size()==1 is not satisfied. Don't you think it should be "!pendingRequest.isEmpty()"?

[Ferdous]

And that's exactly what happens - you get one download.

When one download completes, processNextRequest() is called. So shouldn't the download for next request in pendingRequestList continue successfully?

Yeah, but after the first download you have two elements in the queue, so the condition size()==1 is not satisfied. Don't you think it should be "!pendingRequest.isEmpty()"?

pendingRequestList.size() == 1 condition is meant for only the first download request so that only the first download request gets chance to call processNextRequest as soon as it arrives, for other requests, they are put in the pendingRequestList. When the first download completes, QHttp::done(bool) signal is emitted and slot myHttpRequestDone(bool) gets called. At the end of myHttpRequestDone(bool) function, processNextRequest() is called and now the first request in the current list gets chance to start download by get().

This is my logic to solve the problem - downloading multiple files. But I've failed so far.

Hope I've been able to make you understand my viewpoint.