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. QList : dynamically add and delete objects
Forum Updated to NodeBB v4.3 + New Features

QList : dynamically add and delete objects

Scheduled Pinned Locked Moved Solved General and Desktop
15 Posts 5 Posters 1.6k Views 1 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.
  • O Offline
    O Offline
    ODБOï
    wrote on 27 Nov 2019, 09:13 last edited by
    #1

    Hello,
    I have a QList of Job (simple QObject derived class) objects
    I need to add and remove Job objects dynamially in that QList

    I am not convinced of the correctness of my two methods
    especially my delete Job method

    please have a look

    QList<Job*> m_jobs;
    
    void addJob(QString _nom,QString _fullpath,float _xMax, float _yMax){
     
        Job* job = new Job(this);
        if(!job){return;}
        job->setNom(_nom);
        // ...
    
        m_jobs.append(job);
        emit jobsChanged();
    }
    
    //---
    
    void deleteJob(QString pName, int pIndex){
    
        if(pIndex<0||pIndex>m_selectedPrograms.length()){
            qDebug()<< "can't delete job n° " << pIndex;
            return;
        }
        if(m_jobs.at(pIndex)->nom() == pName){
           
            m_jobs.at(pIndex)->deleteLater();  // < 
            m_jobs.removeAt(pIndex);  // <
            emit jobsChanged();
        }
        return;
    }
    
    J J K J 4 Replies Last reply 27 Nov 2019, 09:19
    0
    • O ODБOï
      27 Nov 2019, 09:13

      Hello,
      I have a QList of Job (simple QObject derived class) objects
      I need to add and remove Job objects dynamially in that QList

      I am not convinced of the correctness of my two methods
      especially my delete Job method

      please have a look

      QList<Job*> m_jobs;
      
      void addJob(QString _nom,QString _fullpath,float _xMax, float _yMax){
       
          Job* job = new Job(this);
          if(!job){return;}
          job->setNom(_nom);
          // ...
      
          m_jobs.append(job);
          emit jobsChanged();
      }
      
      //---
      
      void deleteJob(QString pName, int pIndex){
      
          if(pIndex<0||pIndex>m_selectedPrograms.length()){
              qDebug()<< "can't delete job n° " << pIndex;
              return;
          }
          if(m_jobs.at(pIndex)->nom() == pName){
             
              m_jobs.at(pIndex)->deleteLater();  // < 
              m_jobs.removeAt(pIndex);  // <
              emit jobsChanged();
          }
          return;
      }
      
      J Offline
      J Offline
      jsulm
      Lifetime Qt Champion
      wrote on 27 Nov 2019, 09:19 last edited by
      #2

      @LeLev Looks good, why do you think it could be wrong?

      https://forum.qt.io/topic/113070/qt-code-of-conduct

      O 1 Reply Last reply 27 Nov 2019, 09:30
      6
      • O ODБOï
        27 Nov 2019, 09:13

        Hello,
        I have a QList of Job (simple QObject derived class) objects
        I need to add and remove Job objects dynamially in that QList

        I am not convinced of the correctness of my two methods
        especially my delete Job method

        please have a look

        QList<Job*> m_jobs;
        
        void addJob(QString _nom,QString _fullpath,float _xMax, float _yMax){
         
            Job* job = new Job(this);
            if(!job){return;}
            job->setNom(_nom);
            // ...
        
            m_jobs.append(job);
            emit jobsChanged();
        }
        
        //---
        
        void deleteJob(QString pName, int pIndex){
        
            if(pIndex<0||pIndex>m_selectedPrograms.length()){
                qDebug()<< "can't delete job n° " << pIndex;
                return;
            }
            if(m_jobs.at(pIndex)->nom() == pName){
               
                m_jobs.at(pIndex)->deleteLater();  // < 
                m_jobs.removeAt(pIndex);  // <
                emit jobsChanged();
            }
            return;
        }
        
        J Online
        J Online
        JonB
        wrote on 27 Nov 2019, 09:22 last edited by JonB
        #3

        @LeLev
        @jsulm has beaten me too it. Looks fine, QList does not take ownership of things stored in it, you do your own new & deleteLater.

        The only tiny thing that is probably wrong is:

        if(pIndex<0||pIndex>m_selectedPrograms.length()){
        

        [You like squeezing every space out of your expressions to make them hard to read, do you? :) ] You probably intend pIndex >= m_selectedPrograms.length() [greater-than-or-equal] :)

        1 Reply Last reply
        5
        • O ODБOï
          27 Nov 2019, 09:13

          Hello,
          I have a QList of Job (simple QObject derived class) objects
          I need to add and remove Job objects dynamially in that QList

          I am not convinced of the correctness of my two methods
          especially my delete Job method

          please have a look

          QList<Job*> m_jobs;
          
          void addJob(QString _nom,QString _fullpath,float _xMax, float _yMax){
           
              Job* job = new Job(this);
              if(!job){return;}
              job->setNom(_nom);
              // ...
          
              m_jobs.append(job);
              emit jobsChanged();
          }
          
          //---
          
          void deleteJob(QString pName, int pIndex){
          
              if(pIndex<0||pIndex>m_selectedPrograms.length()){
                  qDebug()<< "can't delete job n° " << pIndex;
                  return;
              }
              if(m_jobs.at(pIndex)->nom() == pName){
                 
                  m_jobs.at(pIndex)->deleteLater();  // < 
                  m_jobs.removeAt(pIndex);  // <
                  emit jobsChanged();
              }
              return;
          }
          
          K Offline
          K Offline
          kshegunov
          Moderators
          wrote on 27 Nov 2019, 09:24 last edited by kshegunov
          #4

          Looks all right.

          Comments:
          if(!job){return;} unless you're catching the specific out of memory exception, this is pretty much superfluous. You could remove it.

          if(pIndex<0||pIndex>m_selectedPrograms.length()){
          I'd simply assert here, i.e.: Q_ASSERT_X(pIndex < 0 || pIndex >= m_selectedPrograms.length(), "Can't delete job");

          m_jobs.at(pIndex) for brevity, you can retrieve the pointer once, although probably the optimizer is going to do that for you. Something like:

          Job * job = m_jobs.at(pIndex); // Qt asserts on out of bounds here anyhow
          if(job->nom() != pName)
              return;
          
          delete job; // You can use also deleteLater(). If you can guarantee there's nothing pending in the even queue for this object, delete is fine too.
          m_jobs.removeAt(pIndex);
          emit jobsChanged();
          

          Read and abide by the Qt Code of Conduct

          J 1 Reply Last reply 27 Nov 2019, 09:29
          6
          • O ODБOï
            27 Nov 2019, 09:13

            Hello,
            I have a QList of Job (simple QObject derived class) objects
            I need to add and remove Job objects dynamially in that QList

            I am not convinced of the correctness of my two methods
            especially my delete Job method

            please have a look

            QList<Job*> m_jobs;
            
            void addJob(QString _nom,QString _fullpath,float _xMax, float _yMax){
             
                Job* job = new Job(this);
                if(!job){return;}
                job->setNom(_nom);
                // ...
            
                m_jobs.append(job);
                emit jobsChanged();
            }
            
            //---
            
            void deleteJob(QString pName, int pIndex){
            
                if(pIndex<0||pIndex>m_selectedPrograms.length()){
                    qDebug()<< "can't delete job n° " << pIndex;
                    return;
                }
                if(m_jobs.at(pIndex)->nom() == pName){
                   
                    m_jobs.at(pIndex)->deleteLater();  // < 
                    m_jobs.removeAt(pIndex);  // <
                    emit jobsChanged();
                }
                return;
            }
            
            J Offline
            J Offline
            J.Hilk
            Moderators
            wrote on 27 Nov 2019, 09:29 last edited by
            #5

            @LeLev seems, fine, as the others said,

            just one point, since you seem to like to use as little text as possible :)

            if(m_jobs.at(pIndex)->nom() == pName){
                    m_jobs.takeAt(pIndex)->deleteLater();
                    emit jobsChanged();
                }
            

            Be aware of the Qt Code of Conduct, when posting : https://forum.qt.io/topic/113070/qt-code-of-conduct


            Q: What's that?
            A: It's blue light.
            Q: What does it do?
            A: It turns blue.

            1 Reply Last reply
            2
            • K kshegunov
              27 Nov 2019, 09:24

              Looks all right.

              Comments:
              if(!job){return;} unless you're catching the specific out of memory exception, this is pretty much superfluous. You could remove it.

              if(pIndex<0||pIndex>m_selectedPrograms.length()){
              I'd simply assert here, i.e.: Q_ASSERT_X(pIndex < 0 || pIndex >= m_selectedPrograms.length(), "Can't delete job");

              m_jobs.at(pIndex) for brevity, you can retrieve the pointer once, although probably the optimizer is going to do that for you. Something like:

              Job * job = m_jobs.at(pIndex); // Qt asserts on out of bounds here anyhow
              if(job->nom() != pName)
                  return;
              
              delete job; // You can use also deleteLater(). If you can guarantee there's nothing pending in the even queue for this object, delete is fine too.
              m_jobs.removeAt(pIndex);
              emit jobsChanged();
              
              J Online
              J Online
              JonB
              wrote on 27 Nov 2019, 09:29 last edited by JonB
              #6

              @kshegunov
              Right, this is helpful, but you're always a stickler for detail, so I'm going to suggest something in your code :)

              delete job; // You can use also deleteLater(). If you can guarantee theres nothing pending in the even queue for this object, delete is fine too.
              m_jobs.removeAt(pIndex);
              

              Why do you delete an object while m_jobs still contains a reference to it, which is invalid for a nanosecond? Why not reverse the lines for good coding practice:

              m_jobs.removeAt(pIndex);
              delete job;
              

              I know I'm struggling here to find a real-world case, but if an "interrupt" arrives between these two statements and code "walks" m_jobs it won't be good. I'm going to put a debugger breakpoint after your delete and before your removeAt(), and get worried when I look at m_jobs in the debugger ;-)

              K 1 Reply Last reply 27 Nov 2019, 09:32
              1
              • J jsulm
                27 Nov 2019, 09:19

                @LeLev Looks good, why do you think it could be wrong?

                O Offline
                O Offline
                ODБOï
                wrote on 27 Nov 2019, 09:30 last edited by
                #7

                thank you so much for your inputs friends ! really appreciate

                @jsulm honestly it's the first time i really need create / delete objects dynamically in my c++ application. I'm not familiar (i will) with pointers and dynamic memory management..

                1 Reply Last reply
                1
                • J JonB
                  27 Nov 2019, 09:29

                  @kshegunov
                  Right, this is helpful, but you're always a stickler for detail, so I'm going to suggest something in your code :)

                  delete job; // You can use also deleteLater(). If you can guarantee theres nothing pending in the even queue for this object, delete is fine too.
                  m_jobs.removeAt(pIndex);
                  

                  Why do you delete an object while m_jobs still contains a reference to it, which is invalid for a nanosecond? Why not reverse the lines for good coding practice:

                  m_jobs.removeAt(pIndex);
                  delete job;
                  

                  I know I'm struggling here to find a real-world case, but if an "interrupt" arrives between these two statements and code "walks" m_jobs it won't be good. I'm going to put a debugger breakpoint after your delete and before your removeAt(), and get worried when I look at m_jobs in the debugger ;-)

                  K Offline
                  K Offline
                  kshegunov
                  Moderators
                  wrote on 27 Nov 2019, 09:32 last edited by
                  #8

                  @JonB said in QList : dynamically add and delete objects:

                  Why do you delete an object while m_jobs still contains a reference to it, which is invalid for a nanosecond? Why not reverse the lines for good coding practice:

                  Because no reason. It was written like this, and I wrote it in the same order. Since we are talking single-threaded here it really means absolutely nothing in what order you delete and remove. That's proper too, because you don't want code that's behaviorally dependent on order. If it were a multithreaded example, then this'd be serialized between the threads, so same considerations apply.

                  Read and abide by the Qt Code of Conduct

                  1 Reply Last reply
                  0
                  • O Offline
                    O Offline
                    ODБOï
                    wrote on 27 Nov 2019, 10:02 last edited by ODБOï
                    #9
                    void deleteJob(QString pName, int pIndex){
                    
                        Q_ASSERT_X(pIndex < 0 || pIndex >= m_jobs.length(), "deleteJob" ,"Can't delete job");
                    
                        if( m_jobs.at(pIndex)->nom() == pName ){
                    
                            m_jobs.takeAt(pIndex)->deleteLater();
                            emit jobsChanged();
                        }
                        return;
                    
                    }
                    

                    @kshegunov i'm little bit confused about Q_ASSERT_X

                    Q_ASSERT_X(pIndex < 0 || pIndex >= m_jobs.length(), "deleteJob" ,"Can't delete job");
                    
                    here  (if pIndex < 0) or  (pIndex >= m_jobs.length()) we want to throw en error
                    

                    or

                    Q_ASSERT_X(pIndex >= 0 &&  pIndex <  m_jobs.length(), "deleteJob" ,"Can't delete job");
                    
                    J 1 Reply Last reply 27 Nov 2019, 10:33
                    0
                    • O ODБOï
                      27 Nov 2019, 10:02
                      void deleteJob(QString pName, int pIndex){
                      
                          Q_ASSERT_X(pIndex < 0 || pIndex >= m_jobs.length(), "deleteJob" ,"Can't delete job");
                      
                          if( m_jobs.at(pIndex)->nom() == pName ){
                      
                              m_jobs.takeAt(pIndex)->deleteLater();
                              emit jobsChanged();
                          }
                          return;
                      
                      }
                      

                      @kshegunov i'm little bit confused about Q_ASSERT_X

                      Q_ASSERT_X(pIndex < 0 || pIndex >= m_jobs.length(), "deleteJob" ,"Can't delete job");
                      
                      here  (if pIndex < 0) or  (pIndex >= m_jobs.length()) we want to throw en error
                      

                      or

                      Q_ASSERT_X(pIndex >= 0 &&  pIndex <  m_jobs.length(), "deleteJob" ,"Can't delete job");
                      
                      J Online
                      J Online
                      JonB
                      wrote on 27 Nov 2019, 10:33 last edited by JonB
                      #10

                      @LeLev said in QList : dynamically add and delete objects:

                      Q_ASSERT_X

                      Haven't you & @kshegunov got your test the wrong way round?? If the condition is false it will assert, i.e. it wants the condition to be true, just like assert. So you need:

                      Q_ASSERT_X(pIndex >= 0 && pIndex < m_selectedPrograms.length(), "delete" ,"Can't delete job");
                      

                      Right, or am I going mad?! :)

                      O 1 Reply Last reply 27 Nov 2019, 10:40
                      2
                      • J JonB
                        27 Nov 2019, 10:33

                        @LeLev said in QList : dynamically add and delete objects:

                        Q_ASSERT_X

                        Haven't you & @kshegunov got your test the wrong way round?? If the condition is false it will assert, i.e. it wants the condition to be true, just like assert. So you need:

                        Q_ASSERT_X(pIndex >= 0 && pIndex < m_selectedPrograms.length(), "delete" ,"Can't delete job");
                        

                        Right, or am I going mad?! :)

                        O Offline
                        O Offline
                        ODБOï
                        wrote on 27 Nov 2019, 10:40 last edited by ODБOï
                        #11

                        @JonB i was trying to figure this out right now,

                        what is strange is that both versions work.. i never see an error message printed.

                        I even tried
                        Q_ASSERT_X(pIndex ==1000 , "deleteProgram" ,"Can't delete job");

                        Q_ASSERT_X(false, "deleteProgram" ,"Can't delete job");

                        and this must be false ! and it should print error message but this still works .. what is going on here how Q_ASSERT_X must work ?

                        J 1 Reply Last reply 27 Nov 2019, 10:42
                        0
                        • O ODБOï
                          27 Nov 2019, 10:40

                          @JonB i was trying to figure this out right now,

                          what is strange is that both versions work.. i never see an error message printed.

                          I even tried
                          Q_ASSERT_X(pIndex ==1000 , "deleteProgram" ,"Can't delete job");

                          Q_ASSERT_X(false, "deleteProgram" ,"Can't delete job");

                          and this must be false ! and it should print error message but this still works .. what is going on here how Q_ASSERT_X must work ?

                          J Online
                          J Online
                          JonB
                          wrote on 27 Nov 2019, 10:42 last edited by JonB
                          #12

                          @LeLev
                          Q_ASSERT_X(false, ... fails the assertion and prints the message.
                          Q_ASSERT_X(true, ... succeeds the assertion and continues.

                          "Assert" means "make sure this is true, else error". Same for Q_ASSERT_X, Q_ASSERT & assert().

                          And for the Qt ones

                          It does nothing if QT_NO_DEBUG was defined during compilation.

                          You won't get anything at all in Release builds, regardless..... They're intended as debug development checks.

                          O 1 Reply Last reply 27 Nov 2019, 10:45
                          0
                          • J JonB
                            27 Nov 2019, 10:42

                            @LeLev
                            Q_ASSERT_X(false, ... fails the assertion and prints the message.
                            Q_ASSERT_X(true, ... succeeds the assertion and continues.

                            "Assert" means "make sure this is true, else error". Same for Q_ASSERT_X, Q_ASSERT & assert().

                            And for the Qt ones

                            It does nothing if QT_NO_DEBUG was defined during compilation.

                            You won't get anything at all in Release builds, regardless..... They're intended as debug development checks.

                            O Offline
                            O Offline
                            ODБOï
                            wrote on 27 Nov 2019, 10:45 last edited by
                            #13

                            @JonB said in QList : dynamically add and delete objects:

                            Q_ASSERT_X(false, ... fails the assertion and prints the message.

                            in my case, right now, it is not..

                            i did

                            void deleteProgram(QString pName, int pIndex){
                            Q_ASSERT_X(false, "deleteProgram" ,"Can't delete job");
                            if( m_selectedPrograms.at(pIndex)->nom() == pName ){
                            
                                    m_selectedPrograms.takeAt(pIndex)->deleteLater();
                            
                                    emit selectedProgramsChanged();
                                }
                            }
                            

                            and program is deleted, no error printed
                            i will try clean/rebuild

                            J 1 Reply Last reply 27 Nov 2019, 10:46
                            0
                            • O ODБOï
                              27 Nov 2019, 10:45

                              @JonB said in QList : dynamically add and delete objects:

                              Q_ASSERT_X(false, ... fails the assertion and prints the message.

                              in my case, right now, it is not..

                              i did

                              void deleteProgram(QString pName, int pIndex){
                              Q_ASSERT_X(false, "deleteProgram" ,"Can't delete job");
                              if( m_selectedPrograms.at(pIndex)->nom() == pName ){
                              
                                      m_selectedPrograms.takeAt(pIndex)->deleteLater();
                              
                                      emit selectedProgramsChanged();
                                  }
                              }
                              

                              and program is deleted, no error printed
                              i will try clean/rebuild

                              J Online
                              J Online
                              JonB
                              wrote on 27 Nov 2019, 10:46 last edited by JonB
                              #14

                              @LeLev
                              See what I've just written about Debug/Release builds! Same with underlying assert() etc. macros, usually (they check for a symbol like DEBUG or _DEBUG, I think).

                              O 1 Reply Last reply 27 Nov 2019, 10:48
                              3
                              • J JonB
                                27 Nov 2019, 10:46

                                @LeLev
                                See what I've just written about Debug/Release builds! Same with underlying assert() etc. macros, usually (they check for a symbol like DEBUG or _DEBUG, I think).

                                O Offline
                                O Offline
                                ODБOï
                                wrote on 27 Nov 2019, 10:48 last edited by
                                #15

                                @JonB said in QList : dynamically add and delete objects:

                                See

                                ok ! thank you i thought i was starting to get crazy..

                                1 Reply Last reply
                                0

                                1/15

                                27 Nov 2019, 09:13

                                • Login

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