Important: Please read the Qt Code of Conduct - https://forum.qt.io/topic/113070/qt-code-of-conduct

Qtimer - Problem to destroy object



  • Hi All,
    Sorry for this stupid question, but i'm unable to understand this behavior. (I'm a newbie with c++ and Qt)
    This is my code :

    void MainWindow::on_ResistenzeOn_clicked()
    {
    
        
        countDown = new QTimer(this);
        //int val = ui->lcdSetTimer->value();
        connect(countDown,SIGNAL(timeout()),this,SLOT(countDownSlot()));
        countDown->start(1000);
        gpioWrite(OUT_RES, 1);
        ui->minutiPiu->setEnabled(false);
        ui->minutiMeno->setEnabled(false);
        ui->labelResistence->setPixmap(MainWindow::fireScaled);
    
    
    }
    
    
    void MainWindow::on_ResistenzeOFF_clicked()
    {
    
        ui->minutiPiu->setEnabled(true);
        ui->minutiMeno->setEnabled(true);
        countDown->stop();
        delete countDown;
        gpioWrite(OUT_RES, 0);
        ui->labelResistence->setPixmap(MainWindow::coldScaled);
    
    }
    
    void MainWindow::countDownSlot() {
        static int setTime = 60;
        if((--setTime) > 0)
            ui->lcdGetResTimer->display(setTime);
        else {
            qDebug() << "sono a zero";
    
        }
    
    }
    
    

    When i click on ResistenzeOn() the countDown start and the lcdGetResTimer display the countdown .. 59 .. 58 .. 57 ... and so on..
    But when i click ResistenzeOFF() the countDown stop, but if click again ResistenzeOn the LcdGetResTimer don't display every number but the number - 2 e.g 55 .. 53 .. 51 ..49 an so on..
    I miss something..
    Someone can help me?
    Thanks,
    Giovanni.


  • Qt Champions 2017

    @ciclonite

    static int setTime = 60;
    

    This line is why it's happening like this. Put the setTime variable in your class as a member variable and set it to its initial value every time you start the timer (in the on_ResistenzeOn_clicked slot).

    Kind regards.



  • In on_ResistenzeOn_clicked you don't check if you already have a QTimer object. If you already have one (because you called on_ResistenzeOn_clicked before but did not call on_ResistenzeOff_clicked) then you'll have 2 running timers. Next time you'll have 3. And because you only store a pointer to one timer (always the latest one you created) you can no longer stop the ones you created previously.



  • @kshegunov Why should this be a problem in this case?


  • Qt Champions 2017

    @Wieland
    Why should what be a problem? If you're talking about the static variable, it shouldn't. However it's not reinitialized in the beginning of each counting cycle, which I inferred was the desired behavior from the problem description, namely: "but if click again ResistenzeOn the LcdGetResTimer don't display every number but the number - 2 e.g 55 .. 53 .. 51 ..49 an so on..". In my opinion this timer is prime candidate for being created as member of the main window, instead of being re-created on every click, and then there's no problem with hanging objects (which you correctly noted in your comment). Something like this:

    class MainWindow : public QMainWindow
    {
         // ...
         QTimer countDown;
    }
    

    or, if @ciclonite still wishes to allocate it in the heap, at least it's better to do it in the constructor:

    class MainWindow : public QMainWindow
    {
        // ...
        QTimer * countDown;
    }
    
    MainWindow::MainWindow()
        : QMainWindow(), countDown(new QTimer)
    {
    }
    
    MainWindow::~MainWindow()
    {
        delete countDown;
    }
    

    Kind regards.



  • @kshegunov You're right with everything you said. Of course this is all messed up and needs to be refactored like you suggested. I just thought you had spotted another, more subtile bug that I haven't noticed. :-)


  • Lifetime Qt Champion

    Hi,

    You can also make MainWindow the parent of the timer so there's no need for the delete call in MainWindow's destructor.


Log in to reply