(seemingly) simple Qt app: creating a progress bar

goetz

In that case I would move the actual computation to a worker thread. Use the pattern that's recommended nowadays: Create a QObject subclass that does the work and QThread as a managing helper only (no subclassing of QThread). The "Threads, Events and QObjects":/wiki/Threads_Events_QObjects wiki article has the details. Your worker object defines a signal, say progress(int value, int max) that you emit, say every 200 iterations of your loop (in every iteration it would just flood the app with events!). The main thread does little less than opening a progress bar (or progress dialog) and spins the event loop for the GUI.

mzimmers

OK...I scanned the article you identified. I was taken by this line:

bq. Nine times out of ten, a quick inspection of their code shows that the biggest problem is the very fact they’re using threads in the first place, and they’re falling in one of the endless pitfalls of parallel programming.

So...am I "doing this wrong?" I'll go this route if you think it's best, but I'm open to other suggestions (as long as they don't entail a complete re-write of my app; the boss won't go for that).

Back to the example I posted originally: would you (hypothetically) use threads for that as well?

goetz

In your case I would say that going with multithreaded architecture is ok. That 9-of-10 statement referes to stuff where you don't need MT at all, like putting an already asynchronous API like QNAM or sockets into a thread to make it asynchronous.

To answer the last question: yes, I would use a thread for this.

mzimmers

OK, thanks, Volker...I guess I've got some reading to do. I appreciate the feedback, as always.

Edit: Volker, I sent you an email with a couple of questions; if you're so inclined to respond, that would be great.

mzimmers

OK, I've done a little work on this. I know it's incomplete, and I'm sure it's not right yet, but...I'm ready to ask for some feedback. Thanks...

@#include <iostream>
#include <fstream>
#include <string>

using namespace std;

#include <QApplication>
#include <QProgressBar>

long getFileSize (fstream& f)
{
long end;

f.seekg (0, ios::end);
end = f.tellg();
f.seekg(0, ios::beg);

return end;
}

class WorkerThread : public QObject
{
Q_OBJECT
private:
fstream myFile;
public:
WorkerThread() : myFile("test.pro.user") {}

void run()
{
long curr, end, percent;
string myStr;

end = getFileSize(myFile);

while (myFile.good())
{
myFile >> myStr;
curr = myFile.tellg();

// special handling for end of file.

if (curr != -1)
percent = curr * 100 / end;
else
percent = 100;

// some kind of signal to be emitted here

cout << percent << " percent finished." << endl;
}
}
signals:
void percentChanged(int percent);
};

int main(int argc, char *argv[])
{
QApplication app (argc, argv);
WorkerThread worker;

QProgressBar bar(0);
bar.setRange(0, 100);
bar.setValue(0);
bar.show();
// bar.setValue(percent);

return app.exec();
}

void WorkerThread::percentChanged(int percent) {}
@

goetz

On a first glance, that looks good. Despite the fact that you might consider using [[Doc:QFile]]/[[Doc:QTextStream]] for the file operations and [[Doc:QFileInfo]] for getting the file size (instead of your method).

Then you must not define the method for the signal yourself, this is done automatically by moc. It is sufficient to just declare the signal in the class header file.

The signal must be emitted in the run method of your thread:

@
// some kind of signal to be emitted here
emit percentChanged(percent);
@

You'll also have to connect that signal to the progress bar:

@
connect(worker, SIGNAL(percentChanged(int)), bar, SLOT(setValue(int)));
@

Also, the worker needs to be moved to a thread, and eventually started:

@
QProgressBar bar(0);
bar.setRange(0, 100);
bar.setValue(0);
bar.show();

WorkerThread worker;
QThread thread;
worker.moveToThread(&thread);

// run the run method of the worker object once the thread has started
connect(&thread, SIGNAL(started())), &worker, SLOT(run()));

// update the progress bar
connect(&worker, SIGNAL(percentChanged(int)), &bar, SLOT(setValue(int)));

thread.start();
@

to make the autostart work, you must declare WorkerThread::run() as public slot.

mzimmers

[quote author="Volker" date="1329477431"]On a first glance, that looks good. Despite the fact that you might consider using [[Doc:QFile]]/[[Doc:QTextStream]] for the file operations and [[Doc:QFileInfo]] for getting the file size (instead of your method)[/quote]

Good to know about those...thanks. For now, I'll just leave it as is. The plan is to:

get this working with a minimum of effort on my sample program

transfer the Qt-specific code to my simulator

let the powers-that-be see how cool even a bit of Qt razzle-dazzle is

go whole-hog on the big job that's hopefully coming up this spring

[quote]Then you must not define the method for the signal yourself, this is done automatically by moc. It is sufficient to just declare the signal in the class header file.[/quote]

Oh yeah; I knew that (sort of).

[quote] The signal must be emitted in the run method of your thread:

@
// some kind of signal to be emitted here
emit percentChanged(percent);
@

bq. You'll also have to connect that signal to the progress bar:

@
connect(worker, SIGNAL(percentChanged(int)), bar, SLOT(setValue(int)));
@

bq. Also, the worker needs to be moved to a thread, and eventually started:

@
QProgressBar bar(0);
bar.setRange(0, 100);
bar.setValue(0);
bar.show();

WorkerThread worker;
QThread thread;
worker.moveToThread(&thread);

// run the run method of the worker object once the thread has started
connect(&thread, SIGNAL(started())), &worker, SLOT(run()));

// update the progress bar
connect(&worker, SIGNAL(percentChanged(int)), &bar, SLOT(setValue(int)));

thread.start();
@

bq. to make the autostart work, you must declare WorkerThread::run() as public slot.[/quote]

Done, done and done. I'm geting a compiler error that "'connect' was not declared in this scope." I'm including QObject; isn't that where connect lives?

EDIT: does the connect need object context? There isn't any in the example on the signals/slots doc page.

mlong

If you're using connect() outside of a class that's a subclass of QObject, you'll need to use the full name of the static method, "QObject::connect()"

goetz

Oh, using QFile and friends is much more easier than the stdlib methods - give it a try! :)

As mark already mentioned, just call QObject::connect(). That method ist static in QObject, so that works without problems.

mzimmers

[quote author="Volker" date="1329495814"]Oh, using QFile and friends is much more easier than the stdlib methods - give it a try! :) [/quote]

I believe you, but...the goal for this exercise is not to show off all of Qt's programming features, but to impress upon some reluctant management that it needn't be invasive to a program's design.

[quote]As mark already mentioned, just call QObject::connect(). That method ist static in QObject, so that works without problems.[/quote]

Cool...that's the first time I've seen that done.

OK...now I'm getting a linker error:

@Undefined symbols:
"vtable for WorkerThread", referenced from:
WorkerThread::WorkerThread()in main.o
@

That doesn't look like a coding problem to me; am I missing a library or something?

goetz

Some missing moc steps.

I would recommend to put the WokerThread class in a separate .h header file (and probably extract the implementation to a .cpp file).

moc is usually not run on the file that contains the main method.

If you have changed your project, do a full rebuild to catch up for all the changes.

mzimmers

Oh...excellent! This is very, very encouraging.

EDIT:

Actually, let's just get to the real thing here: this is the main routine in my application:

@#include <iostream>
#include <string>

#include "modem.h"

using namespace std;

int main ()
{
Modem modem;
bool clockEnable = HIGH;
bool resetFlag = LOW;

int32_t rc = 0;
int32_t i = 0;

while (rc == 0)
{
systemClock.setClock(HIGH);
rc = modem.cycle(clockEnable, resetFlag);

systemClock.setClock(LOW);
modem.cycle(clockEnable, resetFlag); // no need to check rc on low clock

if (++i % 1000 == 0)
cout << "main cycled " << dec << i << " times." << endl;
}

return 0;
}
@

So...do the guts of this become my run() function? And, what's currently in my example main() essentially goes into my app main()? Is that the gist of it?

Also, I'd like to move as much of the progress bar processing out of main(). I know it's only three lines now, but...once I get this working in my app, I'm planning on adding some stuff to the UI. Any reason I shouldn't pursue this?

Thanks...

goetz

Yes, the contents of the main method would go into the worker class. Make the Modem and the flags a class variable, and the rest into the run slot. The singal and emit is just like in the dummy example we had before.

[EDIT: the contents of the main method goes to the class, not the method itself, of course :) ]

mzimmers

OK, thanks, Volker. On a related subject, I've been experimenting with creating a QMainWindow, but evidently, I don't understand the constructors as well as I thought. The line:

@ QProgressBar bar(0);@

Is creating an object of type QProgressBar named bar, and with no parent, right? So why doesn't this:

@ QMainWindow mainWindow;
QProgressBar bar(*mainWindow);
@

Create a QMainWindow, and then a bar whose parent is the QMainWindow? I'm getting a "no matching function" compiler error.

mlong

You'd want @QProgessBar bar(&mainWindow)@

(&foo takes the address of an object, where *foo dereferences a pointer to an object)

But...

That's a bad idea! In this case, QProgressBar is created on the stack. Parenting it to QMainWindow would make QMainWindow attempt to take ownership of it (and, as such, try to delete it upon its own destruction, which would cause all kinds of badness to happen) You never want to call delete() on a stack-based item.

However, the following would be ok:
@
QMainWindow mainWindow;
QProgressBar *bar = new QProgressBar(&mainWindow);
@

goetz

mlong is correct here. In general, for real world applications, you should create QWidget based objects on the heap using new. If you set a parent, you do not need to call delete on those, that's handled by Qt automatically.

Also, you should consider using Qt Designer for creating the UI components. I personally find it much more convenient and easier to use than hard coding that stuff manually.

mzimmers

Thank you both for that. So, as a follow-up question...if it's taboo to create bar on the stack, why is it OK to create mainWindow on the stack? (And I realize this is main(), but I see the reasoning in general.)

And, Volker...I'll go look more closely at Qt Designer for awhile. I assume that whatever I come up with there can be neatly stitched back into my sample application?

EDIT:

I just tried the suggestion above:

@ QMainWindow mainWindow;
QProgressBar *bar = new QProgressBar(&mainWindow);
@

But now the linker is complaining about my second connect() call, which I modified to look like this:

@ QObject::connect(&worker, SIGNAL(percentChanged(int)), *bar, SLOT(setValue(int)));
@

(It's another "no matching function" error; I can provide the text if desired.) What did I do to cause this?

goetz

Ah, you're trapped by the pointer-reference-address-dereferencing confusion.

connect takes a pointer for the first and third argument. Which operator (* or & or nothing) you need to prefix the variable with depends on the type of the variable:

@
// stack based, w1 is an object
QWidget w1;

// pointer to a QWidget
// & gets the address of w1
QWidget w1ptr = &w1;

// reference to a QWidget
// no special operator is needed for the object w1
QWidget &w1ref1 = w1;

// reference to a QWidget
// dereference the w1ptr with * to get the object
QWidget &w1ref2 = *w1ptr;

// call a method:
w1.objectName();
w1ptr->objectName();
w1ref1.objectName();

// ------------------------------

// heap based, w2 is a pointer
QWidget w2 = new QWidget(this);

// pointer to a QWidget
// w2 is a pointer - no conversion needed
QWidget *w2ptr = w2;

// reference to a QWidget
// dereference the w1ptr with * to get the object in both cases
QWidget &w2ref1 = *w2;
QWidget &w2ref2 = *w2ptr;

// call a method:
w2->objectName();
w2ptr->objectName();
w2ref1.objectName();
w2ref2.objectName();
@

So, back to connect:

In our first example, all object were created on the stack, and you had that objects in the variables. To make connect() happy, you need to make them to pointers using operator &.

In your modified code, you already have a pointer to the progress bar, so you do not need an operator at all:

@
QObject::connect(&worker, SIGNAL(percentChanged(int)), bar, SLOT(setValue(int)));
@

Passing *bar would be the very same as passing bar (without operator) in the first version.

I suggest reading some C++ tutorial on that topic. I'm sure they explain this much better than me :-)

mzimmers

Hey, Volker...you explained that just fine. That was just a brain-fade on my part. So, back to my earlier question: if bar needs to be on the heap, why doesn't mainWindow?

Also, as long as I'm throwing out random questions: I notice my program no longer terminates. When I get to the end of file, can I signal a dismissal or something from the worker thread?

Alternatively, I can create a "quit" button or something; at that point, a little text and this thing is ready to go.

Thanks.

EDIT: one more random question: I was experimenting with adding text to the progress bar. (I know I can get it to show the precent completed, but right now, I'm being lazy and just going with "xxx." I'm still cracking the code of the documentation syntax, so I'm not sure how to form this: I tried:
@ QString text = "xxx";
bar->text = text;
@
But that didn't work (presumably because the text property is private). So I tried:
@ QString text = "xxx";
bar->text(text);
@
But that didn't work either. The compiler is giving me an error that I'm having trouble interpreting (something about using "const" in the call). What am I doing wrong here?

mzimmers

Ah...after looking at the doc a bit more closely, I believe I misunderstood the text() function. It appears to be a getter, not a setter, so I can see how that wouldn't have worked. What's less clear is how I do set the text.

Here's my calls to the bar object:

@ bar->setRange(0, 100);
bar->setValue(0);
bar->move(10, 10);
bar->setTextVisible(true);
bar->show();
@

What am I missing here to get the progress text to display? Thanks.