Best practice - exposing QThread to qml

[Eligijus]

Re: QFile copy multiple files progress bar

From my previous topic I decided to move file copying to another thread and increment progress not by copied file count but by copied size.

So this is what I got:

class Copier : public QThread
{
    Q_OBJECT
    Q_PROPERTY(double minimum READ minimum NOTIFY minimumChanged)
    Q_PROPERTY(double maximum READ maximum NOTIFY maximumChanged)
    Q_PROPERTY(double progress READ progress NOTIFY progressChanged)

public:
    Copier(QObject *parent = nullptr);

    Q_INVOKABLE void copy(const QString &sourceFolder, const QString &destFolder);
    double minimum() const;
    double maximum() const;
    double progress() const;

signals:
    void minimumChanged(double minimum);
    void maximumChanged(double maximum);
    void progressChanged(double value);

private:
    QStringList getFolderContents(const QString &sourceFolder) const;
    qint64 fileSize(const QString &sourceFolder, const QStringList &fileList);
    void copyFiles();

    virtual void run() Q_DECL_OVERRIDE;

    QString mSourceFolder;
    QString mDestFolder;
    QStringList mFileList;
    double mMinimum;
    double mMaximum;
    double mProgress;
};

Copier::Copier(QObject *parent)
    : QThread(parent),
      mMinimum(0), mMaximum(0), mProgress(0)
{

}

void Copier::copy(const QString &sourceFolder, const QString &destFolder)
{
    mSourceFolder = sourceFolder;
    mDestFolder = destFolder;
    mFileList = getFolderContents(sourceFolder);
    mMinimum = 0;
    mMaximum = fileSize(sourceFolder, mFileList);
    emit minimumChanged(mMinimum);
    emit maximumChanged(mMaximum);

    if (!isRunning())
        start();
}

double Copier::minimum() const
{
    return mMinimum;
}

double Copier::maximum() const
{
    return mMaximum;
}

double Copier::progress() const
{
    return mProgress;
}

QStringList Copier::getFolderContents(const QString &sourceFolder) const
{
    QStringList fileList;
    std::function<bool(const QString&, const QString&)> folderContents;
    folderContents = [&](const QString &sourceFolder, const QString &destFolder)->bool {
        QDir sourceDir(sourceFolder);

        if (!sourceDir.exists())
            return false;

        foreach (const QFileInfo &fileInfo, sourceDir.entryInfoList(QDir::NoDotAndDotDot | QDir::NoSymLinks
                                                                    | QDir::Dirs | QDir::Files)) {
            QString srcFilePath = fileInfo.filePath();
            QString dstFilePath = destFolder.isEmpty() ? fileInfo.fileName() : destFolder + QDir::separator() + fileInfo.fileName();
            if (fileInfo.isDir()) {
                if (!folderContents(srcFilePath, dstFilePath))
                    return false;
            } else {
                fileList.append(dstFilePath);
            }
        }

        return true;
    };

    if (folderContents(sourceFolder, QString()))
        return fileList;
    else
        return QStringList();
}

qint64 Copier::fileSize(const QString &sourceFolder, const QStringList &fileList)
{
    qint64 total = 0;

    foreach (const QString &fileName, fileList)
        total += QFile(sourceFolder + QDir::separator() + fileName).size();

    return total;
}

void Copier::copyFiles()
{
    qint64 bufferSize = 4194304;
    QByteArray buffer;
    mProgress = 0;
    emit progressChanged(mProgress);

    foreach (const QString &fileName, mFileList) {
        QString srcFilePath = mSourceFolder + QDir::separator() + fileName;
        QString dstFilePath = mDestFolder + QDir::separator() + fileName;
        QFile srcFile(srcFilePath);
        QFile dstFile(dstFilePath);

        QFileInfo dst(dstFilePath);
        QDir dir = dst.dir();
        if (!dst.isDir() && !dir.exists())
            dir.mkpath(dir.path());

        if (srcFile.open(QIODevice::ReadOnly) && dstFile.open(QIODevice::WriteOnly)) {
            qint64 internalBufferSize = srcFile.size() < bufferSize ? srcFile.size() : bufferSize;

            while (1) {
                buffer = srcFile.read(internalBufferSize);
                if (buffer.isEmpty())
                    break;

                dstFile.write(buffer);
                mProgress += internalBufferSize;
                emit progressChanged(mProgress);
            }
            srcFile.close();
            dstFile.close();
        }
    }
}

void Copier::run()
{
    copyFiles();
}

Window {
    visible: true
    width: 640
    height: 480
    title: qsTr("Hello World")


    ProgressBar {
        id: control
        x: 50
        y: 50
        width: 400
        height: 100
        padding: 2
        from: copier.minimum
        to: copier.maximum
        value: copier.progress

        background: Rectangle {
            color: "black"
        }

        contentItem: Item {
            Rectangle {
                width: control.visualPosition * parent.width
                height: parent.height
                color: "red"
            }
        }
    }
    Copier {
        id: copier
    }

    Button {
        anchors.top: control.bottom
        anchors.topMargin: 50
        anchors.right: control.right
        text: "copy"
        onClicked: {
            copier.copy("/home/eligijus/BigDisk/testArea/testArea", "/media/eligijus/usb")
        }
    }
}

Important things to note:
How I expose Copier. I create Copier object in qml and call function copy which starts thread.
Copier is derived from QThread
copying is done in Copier run() reimplemented method.

Bottom line is what I have done a common practice or some kind of a hack?
I'm new to multithreading and I'm really interested in your feedback, comments.

[raven-worx]

@Eligijus
when you don't receive any warnings/errors in the console and everything is working as you expect it, your implementation looks fine ;)

[benlau]

Just my 2 cents. Usually, you should not declare a QML component using QThread type. Because the QML component may be destroyed before the thread completed. It should have a QObject as a bridge to kick off the thread and manage its result.

Moreover, in your case, I think QtConcurrent is a more suitable and easy-to-use solution than declaring your own custom QThread object.

[Eligijus]

@benlau Thanks for your insight. I glanced over QtConcurrent and it didn't seem to be applicable to my problem. I'll take a closer look at it.