Unsolved 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. -
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 theon_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 calledon_ResistenzeOn_clicked
before but did not callon_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?
-
@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. :-)
-
Hi,
You can also make MainWindow the parent of the timer so there's no need for the delete call in MainWindow's destructor.