Skip to content
  • Categories
  • Recent
  • Tags
  • Popular
  • Users
  • Groups
  • Search
  • Get Qt Extensions
  • Unsolved
Collapse
Brand Logo
  1. Home
  2. Qt Development
  3. General and Desktop
  4. Fetch text from a site as QString
Forum Updated to NodeBB v4.3 + New Features

Fetch text from a site as QString

Scheduled Pinned Locked Moved Solved General and Desktop
24 Posts 6 Posters 3.1k Views 4 Watching
  • Oldest to Newest
  • Newest to Oldest
  • Most Votes
Reply
  • Reply as topic
Log in to reply
This topic has been deleted. Only users with topic management privileges can see it.
  • SGaistS Offline
    SGaistS Offline
    SGaist
    Lifetime Qt Champion
    wrote on last edited by
    #14

    In addition to what @JonB wrote, the fact that you pass a parent to your manager object only ensures that it will get destroyed when the parent gets destroyed.

    What you currently have is a variant of memory leak since you create new instances of QNetworkAccessManager every time you call that function and they will only get destroyed when your MyClass instance will as well.

    Interested in AI ? www.idiap.ch
    Please read the Qt Code of Conduct - https://forum.qt.io/topic/113070/qt-code-of-conduct

    1 Reply Last reply
    1
    • R Offline
      R Offline
      realroot
      wrote on last edited by
      #15

      I see thanks.
      If I declare it as QNetworkAccessManager manager; I have errors so I made it like this:

      private:
          QNetworkAccessManager* m_manager = new QNetworkAccessManager(this);
      
      void MyClass::downloadFile() {
         connect(m_manager, &QNetworkAccessManager::finished,
             this, &MyClass::replyFinished);
      
         manager->get(QNetworkRequest(QUrl(
             "https://...file")));
      }
      
      void MyClass::replyFinished(QNetworkReply *reply) {
         if (reply->error() == QNetworkReply::NoError) {
             QByteArray data = reply->readAll();
             QFile file(<file>);
             if (file.open(QIODevice::WriteOnly)) {
                 QTextStream out(&file);
                 out << data;
                 file.close();
                 doSomething(<file>);
             }
         }
         reply->deleteLater();
      }
      
      SGaistS 1 Reply Last reply
      0
      • R realroot

        I see thanks.
        If I declare it as QNetworkAccessManager manager; I have errors so I made it like this:

        private:
            QNetworkAccessManager* m_manager = new QNetworkAccessManager(this);
        
        void MyClass::downloadFile() {
           connect(m_manager, &QNetworkAccessManager::finished,
               this, &MyClass::replyFinished);
        
           manager->get(QNetworkRequest(QUrl(
               "https://...file")));
        }
        
        void MyClass::replyFinished(QNetworkReply *reply) {
           if (reply->error() == QNetworkReply::NoError) {
               QByteArray data = reply->readAll();
               QFile file(<file>);
               if (file.open(QIODevice::WriteOnly)) {
                   QTextStream out(&file);
                   out << data;
                   file.close();
                   doSomething(<file>);
               }
           }
           reply->deleteLater();
        }
        
        SGaistS Offline
        SGaistS Offline
        SGaist
        Lifetime Qt Champion
        wrote on last edited by
        #16

        @realroot said in Fetch text from a site as QString:

        I see thanks.
        If I declare it as QNetworkAccessManager manager; I have errors so I made it like this:

        private:
            QNetworkAccessManager* m_manager = new QNetworkAccessManager(this);
        
        void MyClass::downloadFile() {
           connect(m_manager, &QNetworkAccessManager::finished,
               this, &MyClass::replyFinished);
        
        
        

        Move that connect to the constructor of your class. Otherwise each time you call downloadFile you will create a new connection which means that the slot will be called an additional time.

        Interested in AI ? www.idiap.ch
        Please read the Qt Code of Conduct - https://forum.qt.io/topic/113070/qt-code-of-conduct

        1 Reply Last reply
        2
        • R Offline
          R Offline
          realroot
          wrote on last edited by
          #17

          Then it should be so I believe:

          class MyClass : public QAbstractListModel {
              Q_OBJECT
          public:
              MyClass(QObject *parent = nullptr) : QAbstractListModel(parent) {
                  connect(m_manager, &QNetworkAccessManager::finished,
                          this, &MyClass::downloadFinished);
              }
          private:
              QNetworkAccessManager* m_manager = new QNetworkAccessManager(this);
          
          void MyClass::downloadFile() {
              m_manager->get(QNetworkRequest(QUrl(
                 "https://...file")));
          }
          

          It's working at least.

          1 Reply Last reply
          0
          • R Offline
            R Offline
            realroot
            wrote on last edited by
            #18

            If something is wrong let me know, thanks.

            JonBJ 1 Reply Last reply
            0
            • R realroot

              If something is wrong let me know, thanks.

              JonBJ Offline
              JonBJ Offline
              JonB
              wrote on last edited by JonB
              #19

              @realroot
              It looks reasonable. Although your

              QNetworkAccessManager* m_manager = new QNetworkAccessManager(this);
              

              will work personally I would do the new in the MyClass constructor, to the line above where you have moved the connect() like @SGaist said. But maybe that's just me. In any case I believe your code is now acceptable.

              1 Reply Last reply
              0
              • R realroot has marked this topic as solved on
              • R Offline
                R Offline
                realroot
                wrote on last edited by
                #20

                @JonB Like this?

                public:
                            MyClass(QObject *parent = nullptr) : QAbstractListModel(parent) {
                                m_manager = new QNetworkAccessManager(this);
                                connect(m_manager, &QNetworkAccessManager::finished,
                                        this, &MyClass::downloadFinished);
                            }
                
                Pl45m4P 1 Reply Last reply
                0
                • R realroot

                  @JonB Like this?

                  public:
                              MyClass(QObject *parent = nullptr) : QAbstractListModel(parent) {
                                  m_manager = new QNetworkAccessManager(this);
                                  connect(m_manager, &QNetworkAccessManager::finished,
                                          this, &MyClass::downloadFinished);
                              }
                  
                  Pl45m4P Offline
                  Pl45m4P Offline
                  Pl45m4
                  wrote on last edited by
                  #21

                  @realroot

                  Yeah, better, but why is everything in your header?
                  As you add more to your class, it will become pretty crowded and chaotic.


                  If debugging is the process of removing software bugs, then programming must be the process of putting them in.

                  ~E. W. Dijkstra

                  1 Reply Last reply
                  0
                  • SGaistS Offline
                    SGaistS Offline
                    SGaist
                    Lifetime Qt Champion
                    wrote on last edited by
                    #22

                    C++ now allows to initialize class variable at declaration spot. It's easy and nice for base types however, it can make things harder to read for complex type.

                    Next, unless you have complex logic associated, you should use your class initializer list and after that the constructor. This will help the compiler optimize some things.

                    Interested in AI ? www.idiap.ch
                    Please read the Qt Code of Conduct - https://forum.qt.io/topic/113070/qt-code-of-conduct

                    1 Reply Last reply
                    1
                    • R Offline
                      R Offline
                      realroot
                      wrote on last edited by
                      #23

                      @Pl45m4 Functions ( downloadFile and replyFinished) are not in the header. I did not specify that but I did add MyClass scope e.g. void MyClass::downloadFile().

                      @SGaist So:

                      MyClass(QObject *parent = nullptr) : QAbstractListModel(parent), 
                              m_manager(new QNetworkAccessManager(this)) {
                              connect(m_manager, &QNetworkAccessManager::finished,
                                      this, &MyClass::downloadFinished);
                      }
                      
                      Pl45m4P 1 Reply Last reply
                      0
                      • R realroot

                        @Pl45m4 Functions ( downloadFile and replyFinished) are not in the header. I did not specify that but I did add MyClass scope e.g. void MyClass::downloadFile().

                        @SGaist So:

                        MyClass(QObject *parent = nullptr) : QAbstractListModel(parent), 
                                m_manager(new QNetworkAccessManager(this)) {
                                connect(m_manager, &QNetworkAccessManager::finished,
                                        this, &MyClass::downloadFinished);
                        }
                        
                        Pl45m4P Offline
                        Pl45m4P Offline
                        Pl45m4
                        wrote on last edited by
                        #24

                        @realroot said in Fetch text from a site as QString:

                        @Pl45m4 Functions ( downloadFile and replyFinished) are not in the header. I did not specify that but I did add MyClass scope e.g. void MyClass::downloadFile().

                        Don't know what this means, but might be okay.


                        If debugging is the process of removing software bugs, then programming must be the process of putting them in.

                        ~E. W. Dijkstra

                        1 Reply Last reply
                        0

                        • Login

                        • Login or register to search.
                        • First post
                          Last post
                        0
                        • Categories
                        • Recent
                        • Tags
                        • Popular
                        • Users
                        • Groups
                        • Search
                        • Get Qt Extensions
                        • Unsolved