10

I'm new to StackOverflow and wondering if I'm doing this right:

I'm writing a simple Qt application to test multi-threading (something I am also completely new to). I made a MainWindow that contains widgets, and a class MyThread that subclasses QThread and overrides the run() method.

The application simply displays two buttons, "Start Counter" and "Stop Counter", and a text field. When "start counter" is pressed, a worker thread is created and runs in the background, continuously incrementing a counter in a while loop and signaling the main thread (where the GUI is) with the updated value. When "Stop Counter" is pressed, a signal is sent to the main thread that stops the while loop, and the counter is stopped until "Start Counter" is pressed again.

This works perfectly fine ... but is it the best way? I'm new at this, and read a lot of people saying "don't subclass QThread" and other people saying "subclass QThread", and it's a little bit confusing. If this isn't the best way to implement this sort of thing (run a computationally-intensive loop in a background thread with "start" and "stop" buttons), what is? If I'm doing it wrong, how do I do it right? I don't want to learn the wrong way.

Thank you! And here's the code:

MyThread.h

#ifndef MYTHREAD_H
#define MYTHREAD_H

#include <QThread>
#include <QMutex>

class MyThread : public QThread
{
   Q_OBJECT

public slots:
    void stopRunning();

protected:
   virtual void run();

signals:
   void signalValueUpdated(QString);

private:
    bool isRunning;

};

MyThread.cpp

#include "MyThread.h"
#include <QString>

void MyThread::run()
{
    qDebug("Thread id inside run %d",(int)QThread::currentThreadId());

    static int value=0; //If this is not static, then it is reset to 0 every time this function is called.
    isRunning = 1;
    while(isRunning == 1)
    {
        QString string = QString("value: %1").arg(value++);
        sleep(1/1000); //If this isn't here, the counter increments way too fast and dies, or something; the app freezes, anyway.

        emit signalValueUpdated(string);       
    }            
}

void MyThread::stopRunning()
{
    isRunning = 0;
}

MainWindow.h

#ifndef MAINWINDOW_H
#define MAINWINDOW_H

#include <QApplication>
#include <QPushButton>
#include <QHBoxLayout>
#include <QLineEdit>
#include "MyThread.h"

class MainWindow : public QWidget
{
  Q_OBJECT

  public:
    MainWindow(QWidget *parent = 0);

  private:
    //Widgets
    QHBoxLayout * boxLayout;
    QPushButton * startButton;
    QPushButton * stopButton;
    QLineEdit * lineEdit;

    MyThread thread;
};

#endif

MainWindow.cpp

#include "MainWindow.h"

MainWindow::MainWindow(QWidget *parent) : QWidget(parent)
{
    boxLayout = new QHBoxLayout(this);
    startButton = new QPushButton("Start Counter", this);
    stopButton = new QPushButton("Stop Counter", this);
    lineEdit = new QLineEdit(this);

    boxLayout->addWidget(startButton);
    boxLayout->addWidget(stopButton); 
    boxLayout->addWidget(lineEdit);

    qDebug("Thread id %d",(int)QThread::currentThreadId());

    //When the start button is pressed, invoke the start() method in the counter thread
    QObject::connect(startButton,SIGNAL(clicked()),&thread,SLOT(start()), Qt::QueuedConnection);

    //When the stop button is pressed, invoke the stop() method in the counter thread
    QObject::connect(stopButton,SIGNAL(clicked()),&thread,SLOT(stopRunning()), Qt::QueuedConnection);

    //When the counter thread emits a signal saying its value has been updated, reflect that change in the lineEdit field.
    QObject::connect(&thread,SIGNAL(signalValueUpdated(const QString&)),lineEdit,SLOT(setText(const QString&)), Qt::QueuedConnection);
}
evdc
  • 185
  • 1
  • 3
  • 8
  • 4
    Be careful `sleep(1/1000)` means `sleep(0)`. – masoud Mar 05 '13 at 05:13
  • Good catch, thanks! Changed it to sleep(0.001). – evdc Mar 05 '13 at 05:37
  • 4
    and you still have 0. `sleep` accepts integral, I've not seen any `sleep` which takes fractional number. Use `msleep(1)`to achieve what you want. – ixSci Mar 05 '13 at 06:44
  • 1
    To expand @ixSci's answer, read the following link. There are some subtle gotchas: http://mayaposch.wordpress.com/2011/11/01/how-to-really-truly-use-qthreads-the-full-explanation/ – Throwback1986 Mar 05 '13 at 05:29

2 Answers2

5

Most of the time QThread sub-classing is a wrong way to do threading in Qt. I suggest you to read an article about threads, event loops and other which could give you an idea how to use threads in Qt in a better way. But do not listen to anyone who arguing that there is the only one right way to use QThread. There are 2 ways and while subclassing is not needed in general it could be useful sometimes. You just need to use non-subclassing way until you really need to subclass. In your particular case you don't need subclassing.

ixSci
  • 13,100
  • 5
  • 45
  • 79
  • 1
    So, what exactly is the _disadvantage_ of using subclassing? (i.e., why should I _not_ use it?) It seems to me that it is much easier than creating a separate QThread and a separate worker thread and then doing moveToThread(). At least, when using subclassing it is much easier to understand what's going on. – Gijs van Oort Jun 04 '13 at 08:32
  • @GijsvanOort, the article I mentioned above answers your question. – ixSci Jun 04 '13 at 09:19
2

Replace sleep(1/1000); with msleep(100); Things will be just fine :)

FelixSFD
  • 6,052
  • 10
  • 43
  • 117
Aham
  • 49
  • 1
  • 8