HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

MyTimer based on QTimer

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
qtimerbasedmytimer

Problem

I needed a timer that fires in intervals for a given duration when e.g. a button is pressed. The button can be pressed several times thus I thought it would be easiest to create a new timer for each button press and the timer destroys itself when its duration passed. I wrote this...

Interface:

// timerhandler.h
class TimerHandler {
    public:
        virtual void processTimer()=0; // callback
        void startNewTimer(int interval,int duration);
        virtual ~TimerHandler(){}
};

//timerhandler.cpp
#include "timerhandler.h"
#include "mytimer.h"
void TimerHandler::startNewTimer(int interval,int duration){
    new MyTimer(this,interval,duration);
}


Usage: MainWindow inherits TimerHandler

//MainWindow.cpp
void MainWindow::on_pushButton_start_clicked() { this->startNewTimer(100,2000); }
void MainWindow::processTimer() { std::cout << "TIMER" << std::endl; }


Implementation:

//mytimer.h
#include 
#include 
class TimerHandler;
class MyTimer : public QObject {
        Q_OBJECT
    public:
        friend class TimerHandler;
    private:
        TimerHandler* th;
        QTimer* timer;
        MyTimer(){};
        MyTimer(TimerHandler* th,int interval,int duration);
        ~MyTimer();
    public slots:
        void commitSuicide();
        void on_timer_fired();
};

//mytimer.cpp
MyTimer::MyTimer(TimerHandler* th,int interval,int duration) : th(th) {
    timer = new QTimer(this);
    connect(timer, SIGNAL(timeout()), this, SLOT(on_timer_fired()));
    timer->start(interval);
    QTimer::singleShot(duration,this,SLOT(commitSuicide()));
}
MyTimer::~MyTimer(){ delete timer; }
void MyTimer::commitSuicide(){ delete this; }
void MyTimer::on_timer_fired(){ th->processTimer(); }


...it works, but I wonder if there is an easier way (less code) and it is horribly inflexible. I am a bit stuck when I want to change the code to allow different callbacks (that take different parameters).

I first tried a virtual slot as interface. I think it would work s

Solution

That's a lot of code indeed, and for all that code it is not very flexible: the "slot" name is fixed, you force multiple inheritance on your users, so they can have only one such type of timer - what if I want one to call processThing() and another to do processOtherThing?

You should use the signal/slot mechanism to your advantage - that's what it is there for, and get rid of your interface and class entirely: a single (global/static) function is enough.

Consider something like this (QTimer::singleShot itself is a good example of how you should be doing it):

void limited_timer(int interval, int duration, QObject *target, const char *slot)
{
  QTimer *timer = new QTimer;
  QObject::connect(timer, SIGNAL(timeout()), target, slot);
  QTimer::singleShot(duration, timer, SLOT(deleteLater()));
  timer->start(interval);
}


Usage (in one of your main window's methods):

limited_timer(100, 2000, this, SLOT(processTimer()));

Code Snippets

void limited_timer(int interval, int duration, QObject *target, const char *slot)
{
  QTimer *timer = new QTimer;
  QObject::connect(timer, SIGNAL(timeout()), target, slot);
  QTimer::singleShot(duration, timer, SLOT(deleteLater()));
  timer->start(interval);
}
limited_timer(100, 2000, this, SLOT(processTimer()));

Context

StackExchange Code Review Q#112223, answer score: 4

Revisions (0)

No revisions yet.