QProgressBar in Schleife durchlaufen lassen

[Mefisto]

Hallo!

Ich vermute, die, die mir antworten können, werden schon beim Threadtitel erkannt haben, wo mein Problem liegt. Ich kann mir nämlich nicht vorstellen, dass nicht jeder Anfänger mal über dieses Problem stolpert Leider habe ich aber nichts gefunden, was ich eindeutig als Lösung dieses Problems identifizieren konnte, was aber möglicherweise auch daran liegt, dass ich es nur nicht verstanden habe. Lasst also bitte Nachsicht walten und erklärt mir kurz, wie ich dieses Problem am besten beheben kann. Also folgendes ist passiert:

1. Ich habe eine QT Fenster-Anwendung, mit einer QMainWindow-Klasse.
2. Aus ihr heraus wird ein "Scan"-Dialog erstellt und mit show() angezeigt. Er besteht aus einer QProgressBar und einem Abbrechen-Button.
3. Mithilfe von DialogObjektName->start() wird der "Scanprozess" des Dialogs gestartet, er ist also eine Unterfunktion der Dialog-Klasse
4. In der Funktion start() wird eine Datei Zeile für Zeile ausgelesen und analysiert. Die Datei ist groß, etwa 10.000 Zeilen. Erst wird jede Zeile der Datei in eine QStringList kopiert, das dürfte relativ schnell gehen. Die Analyse läuft mit QRegExp ab und nimmt somit mehr Zeit in Anspruch, dafür wollte ich also fairerweise eine QProgressBar einbinden um frustrierten Anwendern vorzubeugen
5. Ich habe eine Loop von 0 bis zur Anzahl der Einträge in der QStringList.
6. Bei jedem Durchlauf der Schleife wird der Wert in QProgressBar so erhöht, dass beim letzten Durchlauf der Wert auf 100% steht (einfacher Dreisatz + Runden)

So, jetzt das Problem: Die QProgressBar läuft nicht durch, sondern steht während dem Durchlaufen der Schleife (etwa 4 Sekunden) auf 0% und danach auf 100%..

Wie kann ich das am besten beheben?

Vielen Dank im Voraus und liebe Grüße!

[brax]

Ich nehme an, dass Du nicht dafür sorgst, dass irgendwann mal events abgehandelt werden. Darum gibt es auch kein GUI-Refresh. Also entweder den Wert per Signal setzen (setValue ist ein Slot) oder nach setValue ein qApp->processEvents(). Ich empfehle die erste Variante.

[franzf]

Zu dem was brax gesagt hat, möchte ich noch folgendes anmerken:
10000 ist verdammt hoch, jedenfalls als max für ne ProgressBar. Es werden doch sowieso nur ganze Werte angezeigt, wenn du bei min=0, max=10000 einen Wert von 50 setzt, was wird dann angezeigt?
Kurz: Nicht nach jedem Wert der Liste die ProgressBar aktualisieren, sondern höchstens nach list.size()/100 mal (für 1%-Genauigkeit), mMn. reicht bei schnelleren Sachen auch eine Genauigkeit von 5% (Aktualisierung bei 5, 10, 15, ...), und auch wenn QRegExp nicht schnell ist, so ist es doch erheblich schneller als ne ProgressBar neu zu zeichnen, du bremst dich durch übertriebene Genauigkeit also nur selber aus.

[Mefisto]

Ok, danke für beide Tipps! Ich versuche mich gerade in Threads einzulesen, finde es aber noch unglaublich verwirrend. Es ist einfach im Vergleich zu dem, wie ich sonst Programme geschrieben habe, relativ verwirrend. Damit meine ich vorallem auch die Verwendung von Signals und Slots. Ich habe jetzt also folgenden Code, der (nachwievor) nicht läuft:

In der logparserdialog.cpp in der Funkion für die tatsächliche Ausführung der Analyse der Datei:

Code: Alles auswählen

// Vorher wird jede Zeile in QStringList fileContent eingelesen

LogParserDialogProcessing *processor;
connect(processor, SIGNAL(stepDone(int)), this, SLOT(progressPartChanged(int newValue)));
processor->run(fileContent);

Und die Datei logparserdialogprocessing.h:

Code: Alles auswählen

#ifndef LOGPARSERDIALOGPROCESSING_H
#define LOGPARSERDIALOGPROCESSING_H

#include <QStringList>
#include <QThread>
#include <QRegExp>
#include <QtCore>

class LogParserDialogProcessing : public QThread
{
    Q_OBJECT
    signals:
        void stepDone(int);
    public:
        void run(QStringList fileContent);
};

#endif // LOGPARSERDIALOGPROCESSING_H

Und die Datei logparserdialogprocessing.cpp:

Code: Alles auswählen

#include "logparserdialogprocessing.h"

void LogParserDialogProcessing::run(QStringList fileContent) {
    QRegExp lineAnalyser("\\s*<sms.*address=\"(.*)\" date=\"(\\d*)\" type=\"(\\d)\".*body=\"(.*)\".*/>\\s*");
    QList<QStringList> fileAnalyse;
    for(int i=0; i<=fileContent.size();i++) {

        if (lineAnalyser.exactMatch(fileContent.at(i))) {
            lineAnalyser.indexIn(fileContent.at(i));
            QStringList currentLineAnalysed;
            currentLineAnalysed << lineAnalyser.cap(1) << lineAnalyser.cap(2) << lineAnalyser.cap(3);
            fileAnalyse.append(currentLineAnalysed);
        }

        emit stepDone(qFloor(i/fileContent.size()*100));
    }
}

Wenn ich das Programm starte und mit der langen Datei teste stürzt mein Programm ab und ich bekomme den Fehler "QObject::connect: Cannot connect (null)::stepDone(int) to LogParserDialog::progressPartChanged(int newValue)"

Wo der Fehler liegen muss sehe ich, aber ich weiß nicht was der Fehler ist. Vielleicht denke ich auch noch falsch und Threads funktionieren doch ganz anders. Achso und das mit dem selteneren Update der QProgressBar mach ich am Ende, das ist dann ja "kosmetik"..

Vielen Dank schonmal!

[Mefisto]

Ok ich habs gefunden, war ein Flüchtigkeitsfehler. Mit "LogParserDialogProcessing *processor = new LogParserDialogProcessing;" scheint jetzt alles zu gehen, der loop im thread wird auch ausgeführt. Nur leider wird die QProgressBar immer noch nicht aktualisiert und springt von 0 auf 100%...

[brax]

Die Variable processor ist deklariert aber nicht initialisiert.

Code: Alles auswählen

LogParserDialogProcessing *processor; // erzeugt einen Pointer aber kein Objekt
connect(processor, SIGNAL(stepDone(int)), this, SLOT(progressPartChanged(int))); // gibt Dir die Fehlermeldung
processor->run(fileContent); // erzeugt den Crash (Zugriff auf nicht initialisierten Pointer)

Stattdessen:

Code: Alles auswählen

LogParserDialogProcessing *processor = new LogParserDialogProcessing;
connect(processor, SIGNAL(stepDone(int)), this, SLOT(progressPartChanged(int))); 
processor->run(fileContent);

und da ich annehme, dass der processor nach dem run nicht mehr gebraucht wird, leg ihn lieber auf dem Stack an, dann brauchst Du Dir um die Speicherverwaltung keine Gedanken zu machen (im Code oben musst Du irgendwann "delete processor;" aufrufen, sonst gibt ein Speicherloch:

Code: Alles auswählen

LogParserDialogProcessing processor;
connect(&processor, SIGNAL(stepDone(int)), this, SLOT(progressPartChanged(int))); 
processor.run(fileContent);

Desweiteren würde ich das connect direkt auf den setValue slot vom QProgressBar machen (ausgehend davon, dass in progressPartChanged nichts weiter passiert:

Code: Alles auswählen

LogParserDialogProcessing processor;
connect(&processor, SIGNAL(stepDone(int)), this->getProgressBar(), SLOT(setValue(int)));  // oder so ähnlich
processor.run(fileContent);

Beherzige auf jeden Fall den Hinweis von franzf! Also das emit nur alle 100 Einträge oder so....

[EDIT]: Du hast die Implementierung von progressPartChanged nicht gezeigt. Damit der ProgressBar geupdatet wird musst Du ein Signal mit dem setValue Slot vom ProgressBar verbinden und nicht einfach setValue direkt aufrufen.

[Mefisto]

Ok, ich hab alles gemacht von dem. Das heißt mein Code sieht jetzt wie folgt aus:

Code: Alles auswählen

LogParserDialogProcessing processor;
connect(&processor, SIGNAL(stepDone(int)), this->progressBarPart, SLOT(setValue(int)));
processor.run(fileContent);

Der Code lässt sich kompillieren, ausführen und es gibt keinen Fehler. Nur leider läuft die Statusbar auch nicht durch sondern steht wie vorher nach 4-5 Sekunden auf 100%.

[brax]

Hmmm... möglich, dass meine Aussage, dass der ProgressBar automatisch geupdatet wird, wenn man das per Signal/Slot macht einfach mal nicht stimmt, sorry.

Dann geh vielleicht doch zurück zu der Lösung mit dem progressPartChanged(int) SLOT und führe nach dem "progressBarPart->setValue" ein qApp->processEvents() aus. Sorry für meinen Fehler.

[Mefisto]

Klar, möglich ist das, und wäre natürlich kein Problem. Aber ich meine gerade das Problem an einer anderen Stelle gefunden zu haben. Und zwar in der Funktion MeinThread::run(QStringList fileContent):

Code: Alles auswählen

void LogParserDialogProcessing::run(QStringList fileContent) {
    QRegExp lineAnalyser("\\s*<sms.*address=\"(.*)\" date=\"(\\d*)\" type=\"(\\d)\".*body=\"(.*)\".*/>\\s*");
    QList<QStringList> fileAnalyse;
    int overallLines = fileContent.size();
    //qDebug() << overallLines;
    for(int i=0; i<overallLines;i++) {

        if (lineAnalyser.exactMatch(fileContent.at(i))) {
            lineAnalyser.indexIn(fileContent.at(i));
            QStringList currentLineAnalysed;
            currentLineAnalysed << lineAnalyser.cap(1) << lineAnalyser.cap(2) << lineAnalyser.cap(3);
            fileAnalyse.append(currentLineAnalysed);
        }

        //qDebug() << qFloor(i/overallLines*100);

        if((i/50)==(qFloor(i/50))) {
            //qDebug() << qFloor(i/overallLines*100);
            emit stepDone(qFloor(i/overallLines*100));
        }
    }
}

So das ist sie. Ich habe die drei qDebug teile mal auskommentiert, aber jedes einzeln getestet. Das erste (vor der Schleife) gibt "5210" aus, die stimmende Anzahl der Zeilen in der Datei. qDebug zwei und drei geben jeweils (natürlich ziemlich oft) "0" aus. Ich wüsste nicht, was an meiner Rechnung falsch ist - wenn ich annehme i=3000, overallLines=5000 müsste die Rechnung i/overallLines*100 letztendlich doch "60" ergeben?

[brax]

Wenn Du mit Gleitkommazahlen rechnen würdest, wäre Deine Rechnung richtig, Du rechnest aber mit Integers. Da ergibt 3000/5000 einfach mal 0. und 0*100 bleibt 0. Vorschlag: ((double)i)/overallLines*100, dadurch wird i zum double gecastet, was dazu führt, dass der "/"-Operator von double aufgerufen wird. Dann ergibt 3000/5000 auch nicht mehr 0 und alles ist schick.

Bitte sag dann nochmal bescheid, ob das mit dem processEvents notwendig ist oder nicht (interessiert mich einfach).

[Mefisto]

ok, das klingt sehr einleuchtend geändert und jetzt läuft alles, inklusive der Progressbar Vielen Dank für eure Hilfe! Ich brauche jetzt nichtmal mehr einen Thread damit das ganze durchläuft

[franzf]

Deine run() ist schlichtweg falsch! run() hat keine Argumente.
Einen Thread startet man auch nicht über den direkten Aufruf von run (geht auch nicht, ist protected), sondern über start(), das führt dann run() in nem neuen Thread (z.B. pthread) aus.

Du solltest dir auch mal diesen Post bezügl. QThread-Verwendung durchlesen:
http://labs.qt.nokia.com/2010/06/17/you ... -it-wrong/

[brax]

Oh, ich hatte das mit dem QThread total übersehen. Dann funktioniert es natürlich auch nicht, das Objekt auf dem Stack zu erzeugen, weil die Lebenszeit des Threads ja länger sein muss als die aufrufende Methode.

Da Du den Thread nicht wirklich startest (siehe den Hinweis von franzf), gibt es im Moment keine Probleme, wenn Du das aber gefixt hast, musst Du das LogParserDialogProcessing Objekt wieder per new auf dem Heap erzeugen. Dann musst Du aber dafür sorgen, dass das Objekt auch wieder gelöscht wird! Du könntest dafür das Signal finished() des Threads mit dem deleteLater() Slot verbinden.

Wenn Du immer nur einen solchen Thread brauchst, wäre es auch denkbar den LogParserDialogProcessing als Membervariable Deines Dialogs zu haben. Das hätte auch den Vorteil, dass das relativ teure Erzeugen eines Threads nur einmal passieren müsste.