Problems with a downloader-class

[sirswine]

Hello!

We want to extract some information from the web by creating a class "downloader". The problem is that the download works if we calls the member function from main.cpp, but it doesn't work if we call it elsewhere. We are completely out of ideas at the moment and would really appreciate som help! The files are included below.

The hedaer file downloader.h
@#ifndef DOWNLOADER_H
#define DOWNLOADER_H
#include <QObject>
#include <QtNetwork/QNetworkAccessManager>
#include <QtNetwork/QNetworkRequest>
#include <QtNetwork/QNetworkReply>
#include <QUrl>
#include <QDateTime>
#include <QFile>
#include <QDebug>

class Downloader : public QObject
{
Q_OBJECT
public:
explicit Downloader(QObject *parent = 0);
void doDownload(QString);

signals:

public slots:
void replyFinished (QNetworkReply *reply);

private:
QNetworkAccessManager *manager;
};
#endif // DOWNLOADER_H
@

downloader.cpp
@#include "downloader.h"
#include <QString>
#include <QDebug>
#include <iostream>
#include <string>

Downloader::Downloader(QObject *parent) :
QObject(parent)
{
}

void Downloader::doDownload(QString input)
{
QString *url = new QString("http://thetvdb.com/api/GetSeries.php?seriesname=");
url->append(input);

manager = new QNetworkAccessManager(this);
manager->get(QNetworkRequest(QUrl(QUrl::fromUserInput(*url))));

connect(manager,SIGNAL(finished(QNetworkReply*)),this,SLOT(replyFinished(QNetworkReply*)));

}

void Downloader::replyFinished(QNetworkReply *reply)
{
if(reply->error())
{
qDebug() << "ERROR!";
qDebug() << reply->errorString();
}
else
{
qDebug() << reply->header(QNetworkRequest::ContentTypeHeader).toString();
qDebug() << reply->header(QNetworkRequest::LastModifiedHeader).toDateTime().toString();
qDebug() << reply->header(QNetworkRequest::ContentLengthHeader).toULongLong();
qDebug() << reply->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
qDebug() << reply->attribute(QNetworkRequest::HttpReasonPhraseAttribute).toString();

    QFile *file = new QFile("C:\\Scallion\\STORAGE\\serie.txt");
    if(file->open(QFile::Append))
    {
        file->write(reply->readAll());
        file->flush();
        file->close();
    }
    delete file;
}
reply->deleteLater();

}
@

main.cpp. In this content the download works
@#include "mainwindow.h"
#include "downloader.h"
#include <QApplication>
#include <QString>

int main(int argc, char *argv[])
{
QApplication a(argc, argv);

MainWindow w;
w.setWindowTitle("Scallion");
w.show();

Downloader d;
QString *str = new QString("modern+family");
qDebug() << str;
d.doDownload(*str);

return a.exec&#40;&#41;;

}@

but it doesn't work if you put the line of codes in this content in another file, mainwindow.cpp...
(the whole file isn't added, only where the call is done)
@void MainWindow::on_searchFrameButton_clicked()
{
addShowActions();
}

void MainWindow::on_tvshowName_returnPressed()
{
addShowActions();
}
void MainWindow::addShowActions()
{
ui->viewLabel2->hide();
std::string tvshowname = ui->tvshowName->text().toStdString();
//qDebug() << ui->tvshowName->text();
SEARCH searchObject = tvshowname;
bool added = searchObject.get_data();
ui->tvshowName->clear();
//privateLayout();

if (added)
{

    Downloader d;
    QString *str = new QString("modern+family");
    d.doDownload(*str);

    ui->viewLabel2->setText(QString::fromStdString(tvshowname) + " was succesfully added");
    ui->viewLabel2->show();
}
else
{
    ui->viewLabel2->setText(QString::fromStdString(tvshowname) + " was succesfully NOT added");
    ui->viewLabel2->show();
}

}
@

Thanks alot!

[sierdzio]

Don't pass QString as a pointer, it's not necessary (QString is implicitly shared, passing a const reference does not hit the performance).

You also don't need to create a QFile pointer. But it's your choice, of course.

You should, however, create the Downloader as a pointer: right now you create it on a stack, and it most probably goes out of scope (is destroyed) before it can receive the network reply.

Also, make sure the manager is unset before creating a new one:
@
Downloader::Downloader(QObject *parent) :
QObject(parent)
{
manager = NULL;
}

void Downloader::doDownload(QString input)
{
// ...

// WRONG!
manager = new QNetworkAccessManager(this);

// Better:
if (!manager)
    manager = new QNetworkAccessManager(this);

@

[SGaist]

Hi and welcome to devnet,

To add to sierdzio:

@
if (!manager) {
manager = new QNetworkAccessManager(this);
connect(manager,SIGNAL(finished(QNetworkReply*)), SLOT(replyFinished(QNetworkReply*)));
}
@

Otherwise your slot will get called one more time each time you call doDownload

[sirswine]

When we do this we're triggering an exception:

:0: error: Exception at 0x5863a82a, code: 0xc0000005: read access violation at: 0x0, flags=0x0 (first chance)

the new lines for creating the object and calling the function:
@Downloader *d;
QString *str = new QString("chuck");
d->doDownload(*str);@

the new lines in downloader.cpp
@Downloader::Downloader(QObject *parent) :
QObject(parent)
{
manager=NULL;
}

void Downloader::doDownload(QString input)
{
QString *url = new QString("http://thetvdb.com/api/GetSeries.php?seriesname=");
url->append(input);

if(!manager)
    {
          manager = new QNetworkAccessManager(this);
          connect(manager,SIGNAL(finished(QNetworkReply*)),this,SLOT(replyFinished(QNetworkReply*)));
   }
manager->get(QNetworkRequest(QUrl(QUrl::fromUserInput(*url))));

@

any more ideas?

[panosk]

Hi,

Try to initialize the manager in the ctor's initialization list:

@
Downloader::Downloader(QObject *parent) :
QObject(parent), manager(0)
{
}
@

[sierdzio]

[quote author="panosk" date="1389196155"]Hi,

Try to initialize the manager in the ctor's initialization list:

@
Downloader::Downloader(QObject *parent) :
QObject(parent), manager(0)
{
}
@[/quote]

This will introduce a leak. You need to pass the parent to the manager.

[panosk]

Do you actually create the Downloader somewhere?
@
Downloader *d = new Downloader(this);
@

[sirswine]

OMG. Thank you! :D
It solved the problem. Thank you guys!

[panosk]

[quote author="sierdzio" date="1389197269"][quote author="panosk" date="1389196155"]Hi,

Try to initialize the manager in the ctor's initialization list:

@
Downloader::Downloader(QObject *parent) :
QObject(parent), manager(0)
{
}
@[/quote]

This will introduce a leak. You need to pass the parent to the manager.[/quote]

Hi sierdzio,

Why would that be a leak? No memory is allocated for any object and in both cases we will have a null pointer once the ctor exits.

[sierdzio]

[quote author="panosk" date="1389200476"][quote author="sierdzio" date="1389197269"][quote author="panosk" date="1389196155"]Hi,

Try to initialize the manager in the ctor's initialization list:

@
Downloader::Downloader(QObject *parent) :
QObject(parent), manager(0)
{
}
@[/quote]

This will introduce a leak. You need to pass the parent to the manager.[/quote]

Hi sierdzio,

Why would that be a leak? No memory is allocated for any object and in both cases we will have a null pointer once the ctor exits.[/quote]

You initialise the manager without a parent, which means that it will not be automatically deleted when you destroy the Downloader. If you put a deleting code into the destructor, it will sure work. I just wrote that to make ye careful ;)