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