When to delete QNetworkAccessManager pointer?

[veroslav]

Hi all,

I am using QNetworkAccessManager in order to download a file from the Internet in the following way:


void MainWindow::readUrlResource(const QString &url) {
QNetworkRequest request;
request.setUrl(QUrl(url));

QNetworkAccessManager *networkManager = new QNetworkAccessManager(this);

//Get notified when the download completes
connect(networkManager, SIGNAL(finished(QNetworkReply*)),
this, SLOT(urlResourceDownloadCompleted(QNetworkReply*)) );

networkManager->get(request);
}

The function above is part of my main window (inheriting from QMainWindow) and it is what is sent as "this" to QNetworkAccessManager constructor. Also, the urlResourceDownloadCompleted() slot looks like this:


void MainWindow::urlResourceDownloadCompleted(QNetworkR eply *reply) {
//TODO: Handle the case when an error occurs during download

qDebug() << "Download completed signal received";

reply->deleteLater();
}

I am wondering whether I need to explicitly delete networkManager in some way or will QMainWindow (parent to it) take care of this eventually? I will have QMainWindow visible for a long time on screen so if it is it taking care of deletion, I wouldn't want to wait for so long, as I might want to create more networkManagers during this time.

If I need to delete the networkManager pointer manually, which is the best place to do so?

I am just trying my best to avoid potential memory leak due to not deleting networkManager pointer.

Thank you in advance. I am very new to Qt and I have some minor experience in C++ but I am having much fun so far and would like to learn thing properly.

Kind Regards,
Veroslav

[spirit]

I am wondering whether I need to explicitly delete networkManager in some way or will QMainWindow (parent to it) take care of this eventually? I will have QMainWindow visible for a long time on screen so if it is it taking care of deletion, I wouldn't want to wait for so long, as I might want to create more networkManagers during this time.


Qt take care of deleting objects which have parents when a parent will be deleted. So, there won't be memory leak.



If I need to delete the networkManager pointer manually, which is the best place to do so?

I am just trying my best to avoid potential memory leak due to not deleting networkManager pointer.

Thank you in advance. I am very new to Qt and I have some minor experience in C++ but I am having much fun so far and would like to learn thing properly.

Kind Regards,
Veroslav

Why don't you use _one_ access manager for your main window (class member)?

[veroslav]

Thank you for your fast reply, spirit.

I thought about having an instance of it (as a pointer) as a class member but was not sure whether it was a good idea to have it occupying memory throughout program execution as I only needed to use it a few times during that time. Just trying to be as effective as possible.

So I would change it to something like this, if I understood it correctly:


//mainwindow.h
class MainWindow : public QMainWindow {
public:
explicit MainWindow(QWidget *parent = 0);
~MainWindow();
private:
QNetworkAccessManager *networkManager;
};

//mainwindow.cpp
MainWindow::MainWindow(QWidget *parent) : QMainWindow(parent) {
networkManager = new QNetworkAccessManager();
}

MainWindow::~MainWindow() {
delete networkManager;
}

And then just use the pointer in my readUrlResource() function?

Thank you very much.

EDITED:

Or perhaps even better let the Qt deal with deletion:


//mainwindow.cpp
MainWindow::MainWindow(QWidget *parent) : QMainWindow(parent) {
networkManager = new QNetworkAccessManager(this);
}

So no need for destructor.

[spirit]

EDITED:

Or perhaps even better let the Qt deal with deletion:


//mainwindow.cpp
MainWindow::MainWindow(QWidget *parent) : QMainWindow(parent) {
networkManager = new QNetworkAccessManager(this);
}

So no need for destructor.

In this case the access manager will be destroyed when the mainwindow is destroyed. So, you can simply make access manager as a member of the mainwindow.

[veroslav]

Thank you for the confirmation, this is how I am going to implement it now.

Kind Regards,
Veroslav