0

Want to understand the difference in code between the MainWindow and the main.cpp. Specifically, how a chunk of code written exclusively in the main.cpp needs to be modified to be part of the mainwindow.cpp and mainwindow.h.

As an example, I am trying to modify the code from this fine answer to work in MainWindow.

main.cpp

#include <QtWidgets>
#include <QtNetwork>

int main(int argc, char *argv[])
{
    QApplication a(argc, argv);
    //setup GUI (you could be doing this in the designer)
    QWidget widget;
    QFormLayout layout(&widget);
    QLineEdit lineEditName;
    QLineEdit lineEditGender;
    QLineEdit lineEditRegion;
    auto edits = {&lineEditName, &lineEditGender, &lineEditRegion};
    for(auto edit : edits) edit->setReadOnly(true);
    layout.addRow("Name:", &lineEditName);
    layout.addRow("Gender:", &lineEditGender);
    layout.addRow("Region:", &lineEditRegion);
    QPushButton button("Get Name");
    layout.addRow(&button);

    //send request to uinames API
    QNetworkAccessManager networkManager;
    QObject::connect(&networkManager, &QNetworkAccessManager::finished,
                 [&](QNetworkReply* reply){
        //this lambda is called when the reply is received
        //it can be a slot in your GUI window class
        //check for errors
        if(reply->error() != QNetworkReply::NoError){
            for(auto edit : edits) edit->setText("Error");
            networkManager.clearAccessCache();
        } else {
            //parse the reply JSON and display result in the UI
            QJsonObject jsonObject=     QJsonDocument::fromJson(reply->readAll()).object();
            QString fullName= jsonObject["name"].toString();
            fullName.append(" ");
            fullName.append(jsonObject["surname"].toString());
            lineEditName.setText(fullName);
            lineEditGender.setText(jsonObject["gender"].toString());
            lineEditRegion.setText(jsonObject["region"].toString());
        }
        button.setEnabled(true);
        reply->deleteLater();
        });
     //url parameters
    QUrlQuery query;
    query.addQueryItem("amount", "1");
    query.addQueryItem("region", "United States");
    QUrl url("http://uinames.com/api/");
    url.setQuery(query);
    QNetworkRequest networkRequest(url);
    //send GET request when the button is clicked
    QObject::connect(&button, &QPushButton::clicked, [&](){
        networkManager.get(networkRequest);
        button.setEnabled(false);
        for(auto edit : edits) edit->setText("Loading. . .");
    });

    widget.show();
    return a.exec();
}

edit

added the timer part of the same answer; please demonstrate how this version with the timer can done as well

QTimer timer;
QObject::connect(&timer, &QTimer::timeout, [&](){
    networkManager.get(networkRequest);
    button.setEnabled(false);
    for(auto edit : edits) edit->setText("Loading. . .");
});
timer.start(60000); //60000 msecs = 60 secs

I struggle with modifying the networkManager as class members, how to structure the code and how to replace the lambda functions.

If someone can provide all the required modifications for me to gain a better understanding that would be great.

Community
  • 1
  • 1
  • As mentioned in the answer, an option would be to allocate the `networkManager` object on the heap, and keep a pointer to it as a member in your `MainWindow` class. what problems are you getting when doing so? – Mike Sep 22 '16 at 16:01
  • @Mike I am new in C++ and Qt so each of these steps seem daunting. I tried defining the `networkManager` in the header file, then tried creating a new object of it in the `MainWindow.cpp` but dont know how to proceed and calll it. Get many erros. Also for the lambda, I could not find any online resources to guide me. If you could -again- save the day and provide me with a code for me to study I would very much appreciate it. –  Sep 22 '16 at 16:06

2 Answers2

2

You would want to separate out the user interface and the controller (business logic) into separate classes.

The body of main() instantiates the ui and the controller and connects them. A timer that fetches new results every 5 seconds. The timer could be rolled into the Controller, too - I show it separated out as an example of adding functionality to an existing class without modifying it.

main.cpp

// https://github.com/KubaO/stackoverflown/tree/master/questions/into-mainwin-39643510
#include "mainwindow.h"
#include "controller.h"

int main(int argc, char *argv[])
{
   QApplication app{argc, argv};
   MainWindow ui;
   Controller ctl;
   QTimer timer;
   timer.start(5*1000);
   QObject::connect(&timer, &QTimer::timeout, &ctl, &Controller::get);

   QObject::connect(&ctl, &Controller::busy, &ui, [&]{ ui.setState(MainWindow::Loading); });
   QObject::connect(&ui, &MainWindow::request, &ctl, &Controller::get);
   QObject::connect(&ctl, &Controller::error, &ui, [&]{ ui.setState(MainWindow::Error); });
   QObject::connect(&ctl, &Controller::values, &ui, &MainWindow::setFields);
   ui.show();
   return app.exec();
}

The controller knows nothing of the user interface, and deals with processing the requests only. It emits a busy signal every time a request starts being processed.

If you wanted to provide better feedback for multiple active requests, the busy signal would need to be emitted only when there were no requests pending and a new one is added, and an idle signal would be emitted when the last request has finished and there are no more pending ones.

controller.h

#ifndef CONTROLLER_H
#define CONTROLLER_H

#include <QtNetwork>

class Controller : public QObject {
   Q_OBJECT
   QNetworkAccessManager manager{this};
   QNetworkRequest request;
   Q_SLOT void onReply(QNetworkReply *);
public:
   explicit Controller(QObject * parent = nullptr);
   Q_SLOT void get();
   Q_SIGNAL void busy();
   Q_SIGNAL void error(const QString &);
   Q_SIGNAL void values(const QString & name, const QString & gender, const QString & region);
};

#endif // CONTROLLER_H

controller.cpp

#include "controller.h"

Controller::Controller(QObject *parent) : QObject(parent)
{
   QUrlQuery query;
   query.addQueryItem("amount", "1");
   query.addQueryItem("region", "United States");
   QUrl url("http://uinames.com/api/");
   url.setQuery(query);
   request = QNetworkRequest(url);
   connect(&manager, &QNetworkAccessManager::finished, this, &Controller::onReply);
}

void Controller::onReply(QNetworkReply * reply) {
   if (reply->error() != QNetworkReply::NoError) {
      emit error(reply->errorString());
      manager.clearAccessCache();
   } else {
      //parse the reply JSON and display result in the UI
      auto jsonObject = QJsonDocument::fromJson(reply->readAll()).object();
      auto fullName = jsonObject["name"].toString();
      fullName.append(" ");
      fullName.append(jsonObject["surname"].toString());
      emit values(fullName, jsonObject["gender"].toString(), jsonObject["region"].toString());
   }
   reply->deleteLater();
}

void Controller::get() {
   emit busy();
   manager.get(request);
}

The user interface knows nothing of any business logic, it provides an API that's sufficient for the business logic to use it. It can be in one of three states: Normal state where results are visible, Loading state where a busy feedback is shown, and Error state where error information is shown. The setFields slot returns the state to Normal.

mainwindow.h

#ifndef MAINWINDOW_H
#define MAINWINDOW_H

#include <QtWidgets>

class MainWindow : public QWidget {
  Q_OBJECT
  QFormLayout layout{this};
  QLineEdit lineEditName;
  QLineEdit lineEditGender;
  QLineEdit lineEditRegion;
  QPushButton button{"Get Name"};
  QLineEdit * edits[3] = {&lineEditName, &lineEditGender, &lineEditRegion};
public:
  enum State { Normal, Loading, Error };
  explicit MainWindow(QWidget * parent = nullptr);
  Q_SLOT void setFields(const QString & name, const QString & gender, const QString & region);
  Q_SLOT void setState(State);
  Q_SIGNAL void request();
};

#endif // MAINWINDOW_H

mainwindow.cpp

#include "mainwindow.h"

MainWindow::MainWindow(QWidget *parent) : QWidget(parent)
{
   for(auto edit : edits) edit->setReadOnly(true);
   layout.addRow("Name:", &lineEditName);
   layout.addRow("Gender:", &lineEditGender);
   layout.addRow("Region:", &lineEditRegion);
   layout.addRow(&button);
   connect(&button, &QPushButton::clicked, this, &MainWindow::request);
}

void MainWindow::setFields(const QString & name, const QString & gender, const QString & region) {
   setState(Normal);
   lineEditName.setText(name);
   lineEditGender.setText(gender);
   lineEditRegion.setText(region);
}

void MainWindow::setState(MainWindow::State state) {
   if (state == Normal) {
      for (auto edit : edits) edit->setEnabled(true);
      button.setEnabled(true);
   }
   else if (state == Loading) {
      for (auto edit : edits) edit->setEnabled(false);
      button.setEnabled(false);
   }
   else if (state == Error) {
      for (auto edit : edits) edit->setText("Error...");
      button.setEnabled(true);
   }
}
Kuba hasn't forgotten Monica
  • 95,931
  • 16
  • 151
  • 313
  • That is great, I now see most of my shortcomings + I see the importance of separating the designs. Could you possibly also add the version with the timer as it can be seen in the other answer's edit (http://stackoverflow.com/a/39521698)? Tried to follow your example but wasn't sure if i had to create a new private slot for it.. –  Sep 23 '16 at 07:56
  • @nk-fford If you create the timer in the `MainWindow` class, you could just connect the timer's `QTimer::timeout` signal to `MainWindow::request` signal. – thuga Sep 23 '16 at 09:10
  • @thuga Could you please show the code (maybe edit the answer) to show how you mean cause I tried this but I get but get errors everywhere.. I am qt noob –  Sep 23 '16 at 09:23
  • @nk-fford Which part is confusing you? Connecting a signal to a signal? Starting the timer? You connect it exactly the same as the button, you just use the timer object and the `QTimer::timeout` signal. – thuga Sep 23 '16 at 09:33
  • @thuga So, in mainwindow.h I replaced the private slot for the button with a onTimeout. In mainwindow.cpp before the `connect` I created a QTimer object and started it and right after that I used that object to define the `connect(&timer,&QTimer::timeout,this,&MainWindow::onTimeout);`. Lastly, i replaced again the button slot definition at the end with the onTimeout stuff. But i do not get any data. (I used froggato's version for simplicity). Please help i am lost –  Sep 23 '16 at 10:17
  • @nk-fford The project is available on github, you can download it and open up in Qt Creator. There's a timer and an easier to use API that facilitates this. – Kuba hasn't forgotten Monica Sep 23 '16 at 13:12
  • @Kuba Your answer is and notes provide me great service in studying Qt. Great stuff. –  Sep 23 '16 at 20:50
  • @Kuba Just a follow up question; could you please explain the ` QFormLayout layout{this};` in `mainwindow.h`? Also, in the interest of good programming practices, is this why you opted to define all GUI elements (layouts and widgets) in that header file instead of the `mainwindow.cpp`? Finally, please consider looking at my other related question http://stackoverflow.com/questions/39669961/designing-gui-programmatically-good-practice-for-nested-layouts-and-multiple-w. –  Sep 24 '16 at 10:17
  • `QFormLayout layout{this}` is a C++11 uniform initialization syntax: a member named `layout` is constructed and given `this` (the widget) as a parameter. This uses the `QFormLayout(QWidget*)` constructor and is the simplest way of setting a layout on a widget. – Kuba hasn't forgotten Monica Sep 25 '16 at 02:32
  • All the gui elements are in the header because they are held by value. They are initialized in the header in the interest of keeping it short. You could of course initialized them in the constructor in the `.cpp` file instead, using initializer list. Generally speaking don't hold things by a pointer unless you have a clearly understood reason for it. Qt examples are not such a reason. – Kuba hasn't forgotten Monica Sep 25 '16 at 02:34
  • I am getting a 'cannot specify explicit initializer for arrays in line: `QLineEdit * edits[3] = {&lineEditName, &lineEditGender, &lineEditRegion};` (using MSVC 2013)`. Do you know a workaround or perhaps suggest an alternative way instead of using the arrays? –  Sep 27 '16 at 10:48
1

You can put all of this code in constructor of your QMainWindow and retaining lambda functions as-is.

Another more clean way would be transforming those lambda functions into private slots. Using this way, you should define networkManager as a class member of your QMainWindow class and also it should be allocated in heap memory not stack. To get this done, just define a QNetworkManager* class member and initialize it in your QMainWindow constructor.

this->networkManager = new QNetworkManager(this);

Once it's been initialized, you can use it in all slots of your QMainWindow class.

A simple rule of thumb is: all shared variables among lambda functions and the main scope should be class members in this way.


The code. (I tested it and works fine)

main.cpp

#include "mainwindow.h"
#include <QApplication>

int main(int argc, char *argv[])
{
    QApplication a(argc, argv);
    MainWindow w;
    w.show();

    return a.exec();
}

mainwindow.cpp

#include "mainwindow.h"
#include "ui_mainwindow.h"

MainWindow::MainWindow(QWidget *parent) :
    QMainWindow(parent),
    ui(new Ui::MainWindow)
{
    ui->setupUi(this);

    ui->lineEditGender->setReadOnly(true);
    ui->lineEditRegion->setReadOnly(true);
    ui->lineEditName->setReadOnly(true);

    networkManager = new QNetworkAccessManager(this);

    connect(networkManager, &QNetworkAccessManager::finished, this, &MainWindow::onNetworkManagerFinished);
    connect(ui->btnGetName, &QPushButton::clicked, this, &MainWindow::onBtnGetNameClicked);
}

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

void MainWindow::onNetworkManagerFinished(QNetworkReply *reply)
{
    if(reply->error() != QNetworkReply::NoError){
        ui->lineEditName->setText("Error");
        ui->lineEditGender->setText("Error");
        ui->lineEditRegion->setText("Error");

        networkManager->clearAccessCache();
    } else {
        //parse the reply JSON and display result in the UI
        QJsonObject jsonObject = QJsonDocument::fromJson(reply->readAll()).object();
        QString fullName= jsonObject["name"].toString();
        fullName.append(" ");
        fullName.append(jsonObject["surname"].toString());
        ui->lineEditName->setText(fullName);
        ui->lineEditGender->setText(jsonObject["gender"].toString());
        ui->lineEditRegion->setText(jsonObject["region"].toString());
    }
    ui->btnGetName->setEnabled(true);
    reply->deleteLater();
}

void MainWindow::onBtnGetNameClicked()
{
    QUrlQuery query;
    query.addQueryItem("amount", "1");
    query.addQueryItem("region", "United States");
    QUrl url("http://uinames.com/api/");
    url.setQuery(query);
    QNetworkRequest networkRequest(url);

    //send GET request when the button is clicked
    networkManager->get(networkRequest);
    ui->btnGetName->setEnabled(false);

    ui->lineEditName->setText("Loading. . .");
    ui->lineEditGender->setText("Loading. . .");
    ui->lineEditRegion->setText("Loading. . .");
}

mainwindow.h

#ifndef MAINWINDOW_H
#define MAINWINDOW_H

#include <QMainWindow>
#include <QNetworkAccessManager>
#include <QNetworkReply>
#include <QNetworkRequest>
#include <QUrlQuery>
#include <QJsonDocument>
#include <QJsonObject>

namespace Ui {
class MainWindow;
}

class MainWindow : public QMainWindow
{
    Q_OBJECT

public:
    explicit MainWindow(QWidget *parent = 0);
    ~MainWindow();

private slots:
    void onNetworkManagerFinished(QNetworkReply* reply);
    void onBtnGetNameClicked();

private:
    Ui::MainWindow *ui;
    QNetworkAccessManager *networkManager;
};

#endif // MAINWINDOW_H
frogatto
  • 28,539
  • 11
  • 83
  • 129
  • Could you please be a bit more explicit cause I am a newbie in all this? For example I put everything (besides the `main` definition and the `QApplication a(argc, argv)`) under the `ui->setupUi(this);` line in the `mainwindow.cpp`. I get so many errors.. Please could you show the code for me to learn from? –  Sep 22 '16 at 18:55
  • @nk-fford Ok. I'll write here the whole code for you to learn how to code. Just let me know do you know how to design the UI in Designer? – frogatto Sep 22 '16 at 19:26
  • I appreciate it. Yes I can make the GUI in the designer. –  Sep 22 '16 at 19:38
  • the `networkManager` object is being leaked, please instantiate it using `networkManager = new QNetworkAccessManager(this);` so that it gets deleted when the `MainWindow` gets destructed. Also, the `readOnly` properties are better done in the designer. Otherwise, I think that this should be it, thanks @HiI'mFrogatto. – Mike Sep 22 '16 at 22:18
  • `QNetworkManager` can be allocated on the stack safely, like in Kuba's answer below. That way you don't get memory leaks even if you forget to assign a parent to it. – thuga Sep 23 '16 at 07:22
  • That is great, I now see most of my shortcomings. Could you possibly also add the version with the timer as it can be seen in the other answer's edit (http://stackoverflow.com/a/39521698)? Tried to follow your example but wasn't sure if i had to create a new private slot for it.. –  Sep 23 '16 at 07:50
  • @Mike You're right, thanks for the point. However if we assume it's the main window of the app then we wouldn't have memory leak because when it's closed the process is finished and all memory gets back to OS. Nevertheless, it's far better to assign a parent for that object. Regarding `readOnly` my intention was to keep the original code untouched where possible (excluding designing the UI with Designer). – frogatto Sep 23 '16 at 08:16
  • 1
    @nk-fford Please accept Kuba's answer, it's better than mine. Always try to separate stuff into single-responsible classes. [Single responsibility principle](https://en.wikipedia.org/wiki/Single_responsibility_principle). – frogatto Sep 23 '16 at 08:19
  • Nevertheless, i appreciate your time and response. Very good lesson for what i was looking to understand. Please consider editing your answer to include code for how the timer can be implemented in your answer. Thanks! –  Sep 23 '16 at 08:22