Qt - Reddit 4024 - Telegram Web

Hospital Project Updates

https://redd.it/1nsyif9
@qt_reddit

2 views20:32

Critique my C++/Qt Task Framework - Is this a bad design?

Hey r/QtFramework,

I've implemented a small framework for running asynchronous/background tasks in my Qt application and I'm looking for some feedback on the design. I'm a bit concerned I might be doing something that's considered an anti-pattern or has hidden dangers.

My main goals were:

* Run jobs in a background thread pool.
* Get signals back on the main thread for completion (success/failure).
* Some tasks could be fire & forget.
* Keep the tasks mockable for unit tests.

Because of the mocking requirement, I steered clear of `QtConcurrent::run` and created my own `QRunnable`\-based system.

# The Design

The core of the framework is an abstract base class that inherits from both `QObject` and `QRunnable`:

`AbstractTask.hpp`

#include <QObject>
#include <QRunnable>
#include <QString>
#include <QVariant>

class AbstractTask : public QObject, public QRunnable {
Q_OBJECT

public:
explicit AbstractTask(QObject* parent = nullptr) {
// Per-task memory management.
setAutoDelete(false);
}

// The main execution wrapper, handles signals and exceptions.
void run() override final {
emit started();
try {
if (execute()) {
emit finished(true, m_errorMessage, m_result);
} else {
emit finished(false, m_errorMessage, m_result);
}
} catch (const std::exception& e) {
m_errorMessage = e.what();
emit finished(false, m_errorMessage, m_result);
}
}

signals:
void started();
void finished(bool success, const QString& errorMessage, const QVariant& result);

protected:
// Concrete tasks implement their logic here.
virtual bool execute() = 0;

// Helpers for subclasses
void setResult(const QVariant& result) { m_result = result; }
void setError(const QString& errorMessage) { m_errorMessage = errorMessage; }

private:
QString m_errorMessage;
QVariant m_result;
};

A concrete task looks like this:

class MyConcreteTask : public AbstractTask {
/* ... constructor, etc. ... */
protected:
bool execute() override {
// Do background work...
if (/* success */) {
setResult(42);
return true;
} else {
setError("Something went wrong");
return false;
}
}
};

And this is how I use it:

void SomeClass::startMyTask() {
auto* task = new MyConcreteTask();

// Connect signals to handle results on the main thread
connect(task, &MyConcreteTask::finished, this, &SomeClass::handleTaskFinished);

// IMPORTANT: Manage the object's lifetime
connect(task, &MyConcreteTask::finished, task, &QObject::deleteLater);

// Run it
QThreadPool::globalInstance()->start(task);
}

# My Specific Concerns:

1. **Inheriting** `QObject` **and** `QRunnable`: This seems to be the standard way to get signals from a `QRunnable`, but is it a good practice?
2. **Memory Management**: I'm explicitly calling `setAutoDelete(false)`. My understanding is that this is necessary because the default auto-deletion can cause a crash if the task finishes and is deleted before its signals are processed. By connecting `finished` to `deleteLater`, I'm ensuring the task is safely deleted on its "home" thread (the main thread) after all signals are emitted. Is this logic sound?
3. `QtConcurrent` **Alternative**: I know `QtConcurrent` is often recommended. My main issue with it is the difficulty in mocking the free function `QtConcurrent::run`. My `AbstractTask` interface is easy to mock in tests. Is there a modern `QtConcurrent` pattern that's more test-friendly that I'm missing?
4. **General "Code Smell"**: Does this whole approach feel right to you? Or

3 views08:32