Qtimer - Problem to destroy object

[ciclonite]

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.

[kshegunov]

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

[A Former User]

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.

[A Former User]

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

[kshegunov]

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

[A Former User]

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

[SGaist]

Hi,

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