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.
  • ODБOïO ODБOï

    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.HilkJ Offline
    J.HilkJ Offline
    J.Hilk
    Moderators
    wrote on 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
    • kshegunovK kshegunov

      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();
      
      JonBJ Offline
      JonBJ Offline
      JonB
      wrote on 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 ;-)

      kshegunovK 1 Reply Last reply
      1
      • jsulmJ jsulm

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

        ODБOïO Offline
        ODБOïO Offline
        ODБOï
        wrote on 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
        • JonBJ JonB

          @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 ;-)

          kshegunovK Offline
          kshegunovK Offline
          kshegunov
          Moderators
          wrote on 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
          • ODБOïO Offline
            ODБOïO Offline
            ODБOï
            wrote on 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");
            
            JonBJ 1 Reply Last reply
            0
            • ODБOïO ODБOï
              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");
              
              JonBJ Offline
              JonBJ Offline
              JonB
              wrote on 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?! :)

              ODБOïO 1 Reply Last reply
              2
              • JonBJ JonB

                @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?! :)

                ODБOïO Offline
                ODБOïO Offline
                ODБOï
                wrote on 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 ?

                JonBJ 1 Reply Last reply
                0
                • ODБOïO ODБOï

                  @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 ?

                  JonBJ Offline
                  JonBJ Offline
                  JonB
                  wrote on 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.

                  ODБOïO 1 Reply Last reply
                  0
                  • JonBJ JonB

                    @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.

                    ODБOïO Offline
                    ODБOïO Offline
                    ODБOï
                    wrote on 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

                    JonBJ 1 Reply Last reply
                    0
                    • ODБOïO ODБOï

                      @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

                      JonBJ Offline
                      JonBJ Offline
                      JonB
                      wrote on 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).

                      ODБOïO 1 Reply Last reply
                      3
                      • JonBJ JonB

                        @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).

                        ODБOïO Offline
                        ODБOïO Offline
                        ODБOï
                        wrote on 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

                        • Login

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