PDA

View Full Version : Pure Callback instead of Signal/Slot



lamb
13th September 2012, 22:54
Hi,

I would like to have your feedback about my source code with a pure callback function.

Do you see any problems here?

MainWindow.h


#ifndef MAINWINDOW_H
#define MAINWINDOW_H

#include <QMainWindow>

class ValueSimulator;

namespace Ui {
class MainWindow;
}

class MainWindow : public QMainWindow
{
Q_OBJECT

public:
explicit MainWindow(QWidget *parent = 0);
~MainWindow();

static void getValues(void *obj, float value);

void setTextForValues(const QString& text);

private slots:
void slotStart();
void slotStop();

private:
Ui::MainWindow *ui;

ValueSimulator* m_pValueSimulator;
};

#endif // MAINWINDOW_H


MainWindow.cpp


#include "MainWindow.h"
#include "ui_MainWindow.h"

#include "ValueSimulator.h"

#include <QDebug>

MainWindow::MainWindow(QWidget *parent) :
QMainWindow(parent),
ui(new Ui::MainWindow)
{
ui->setupUi(this);

m_pValueSimulator = new ValueSimulator(this, &getValues);

connect(ui->StartPushButton, SIGNAL(clicked()), this, SLOT(slotStart()));
connect(ui->StopPushButton, SIGNAL(clicked()), this, SLOT(slotStop()));
}


MainWindow::~MainWindow()
{
delete ui;

delete m_pValueSimulator;
}


void MainWindow::getValues(void* obj, float value)
{
QString msg = QString("%1").arg(value);

qDebug() << msg;

MainWindow* mySelf = (MainWindow*) obj;

mySelf->setTextForValues(msg);
}


void MainWindow::setTextForValues(const QString &text)
{
ui->textEdit->setText(text);
}


void MainWindow::slotStart()
{
m_pValueSimulator->start();
}


void MainWindow::slotStop()
{
m_pValueSimulator->stop();
}


ValueSimulator.h


#ifndef VALUESIMULATOR_H
#define VALUESIMULATOR_H

#include <QObject>

class QTimer;

class ValueSimulator;
typedef void (*SetValues) (void* obj, float);

class ValueSimulator : public QObject
{
Q_OBJECT
public:
ValueSimulator(void* obj, SetValues setValues, QObject* parent = 0);

void start(void);

void stop(void);

private slots:
void slotSetValues(void);

private:
QTimer* m_Timer;

SetValues m_SetValues;

void* m_Object;
};

#endif // VALUESIMULATOR_H



ValuesSimulator.cpp



#include "ValueSimulator.h"

#include <time.h>

#include <QTimer>


ValueSimulator::ValueSimulator(void *obj, SetValues setValues, QObject *parent) : QObject(parent)
{
m_Timer = new QTimer();
m_Timer->setInterval(1000);

m_Object = obj;

m_SetValues = setValues;

connect(m_Timer, SIGNAL(timeout()), this, SLOT(slotSetValues()));

srand ( time(NULL) );
}


void ValueSimulator::start(void)
{
m_Timer->start();
}


void ValueSimulator::stop()
{
m_Timer->stop();
}


void ValueSimulator::slotSetValues()
{
(*m_SetValues)(m_Object, (float)rand() / RAND_MAX);

m_Timer->setInterval(rand() % 1000);
}


Regards,

- the lamb

amleto
13th September 2012, 23:25
why? why do you have all of those void* all over the shop?

I'm guessing you're doing this to get around a circular reference? Otherwise why not just use signal/slot?

In short, whilst the code may work, it isn't very good.

wysota
13th September 2012, 23:30
Hi,

I would like to have your feedback about my source code with a pure callback function.

Do you see any problems here?

Callbacks are Evil!

lamb
14th September 2012, 10:28
Thank you for the answers.

Amleto wrote:


why? why do you have all of those void* all over the shop?

I'm guessing you're doing this to get around a circular reference? Otherwise why not just use signal/slot?

In short, whilst the code may work, it isn't very good.


I need these void* pointers to pass the receiver object, which is in this case "MainWindow". In


void MainWindow::getValues(void* obj, float value)
{
QString msg = QString("%1").arg(value);

qDebug() << msg;

MainWindow* mySelf = (MainWindow*) obj;

mySelf->setTextForValues(msg);

I cast the void pointer to the receiver object "MainWindow" back to call the public function "setTextForValues". And the function MainWindow::getValues(...) must be static otherwise this callback doesn't work.

And of course I could implement the signal/slot mechanism in ValueSimulater but this class should be have a Non-Qt Interface. This class should simulate a pure C/C++ to the outside. OK, inside ValueSimulater there is Qt code (QTimer), but this is the only Qt class which I use, because I don't know how to implement a timer with pure C/C++.

And the code is working without any problems. But I'm not really happy with this void pointers to get the "MainWindow" object back. I don't know a better solution.

- The lamb

Added after 7 minutes:

Wysota wrote:

Callbacks are Evil!

Yes you are right. From the Qt Assistant 4.8.1


Callbacks have two fundamental flaws: Firstly, they are not type-safe. We can never be certain that the processing function will call the callback with the correct arguments. Secondly, the callback is strongly coupled to the processing function since the processing function must know which callback to call.


Do you know a better solution of a callback mechanism with pure C/C++ code (Hence no Qt's Signal/Slot)?
Please tell me your thoughts.

- The lamb

wysota
14th September 2012, 11:21
I need these void* pointers to pass the receiver object, which is in this case "MainWindow".
No, you can pass "MainWindow*" and not "void*".


I cast the void pointer to the receiver object "MainWindow" back to call the public function "setTextForValues". And the function MainWindow::getValues(...) must be static otherwise this callback doesn't work.
Then don't use a callback!


Do you know a better solution of a callback mechanism with pure C/C++ code (Hence no Qt's Signal/Slot)?

Of course.


class GetValuesIface {
public:
virtual ~GetValuesIface() {}
virtual void getValues(float value) = 0; // pure virtual
};

class MainWindow : public QMainWindow, public GetValuesIface {
public:
// ...
void getValues(float value) {
setTextForValues(value);
}
};

and then:


void X::addGetValuesListener(GetValuesIface *obj) {
m_getValueListeners << obj;
}

and:


void X::slotSetValues() {
float val = rand() / RAND_MAX; // note it is not really a float but an int.
foreach(GetValuesIface *iface, m_getValueListeners) iface->getValues(val);
}

ChrisW67
14th September 2012, 12:08
You can use the C++ TR1 functional stuff to marginally improve the situation by having the compiler warn you about incompatible callbacks and provide a default. Example:


#include <iostream>
#include <string>
#include <tr1/functional>

// CallbackFunc is any callable entity that can be called with a std::string
// (or anything compatible with it) that returns anything compatible with an int.
typedef std::tr1::function<int (const std::string&)> CallbackFunc;

int defaultCallback(const std::string& s) {
std::cout << "Default callback" << std::endl;
return s.length();
}

int anotherCallback(const std::string& s) {
std::cout << "Another callback" << std::endl;
return 2 * s.length();
}

int badCallback(int b) {
std::cout << "Bad callback" << std::endl;
return 2 * b;
}

class Thingy {
public:
Thingy(CallbackFunc cb = defaultCallback): callback(cb) { }

void setCallback(CallbackFunc cb) { callback = cb; }

void useCallback() {
int retval = callback("test");
std::cout << retval << std::endl;
}

private:
CallbackFunc callback;
};


int main(void)
{
Thingy t;
t.useCallback();

t.setCallback(anotherCallback);
t.useCallback();

// t.setCallback(badCallback); // compiler will not allow this

return 0;
}

lamb
14th September 2012, 18:11
Thank you "ChrisW67". I right know reading the TR1 and the "std::tr1::function". I will check your example code.

- The lamb

Wysota wrote:

No, you can pass "MainWindow*" and not "void*".
I can't pass "MainWindow*" into ValueSimulator, because this class must be independent of MainWindow. ValueSimulator should be a Non-Qt Class. I'm sorry that I use QTimer in this class.

But your solution with the "X" class is good...

MainWindow implements the interface "GetValueIFace" and "X" hold all these kind of interfaces to send values when the time has come ( in X::slotSetValues() ).

And X is Qt code free (except with the QTimer).

Thank you.

- The lamb

d_stranz
15th September 2012, 04:03
I can't pass "MainWindow*" into ValueSimulator, because this class must be independent of MainWindow

Your ValueSimulator class does not need to know the interface that MainWindow uses in order to talk to it, because you are doing it through a pointer to a "SetValues" callback. The SetValues callback function does need to know the interface. To avoid the type casting from void * and get some type safety into your actual SetValues implementation, all you really have to do is declare a forward reference to MainWindow:



// ValueSimulator.h:

class MainWindow;
typedef void (*SetValues) (MainWindow * obj, float);

class ValueSimulator : public QObject
{
Q_OBJECT
public:
ValueSimulator( MainWindow * obj, SetValues setValues, QObject* parent = 0);

void start(void);

void stop(void);

private slots:
void slotSetValues(void);

private:
QTimer* m_Timer;

SetValues m_SetValues;

MainWindow * m_Object;
};


This will compile just fine, and you don't need to #include "MainWindow.h" into ValueSimulator.cpp because it never actually uses the pointer for anything, it just passes it into the SetValues() call without dereferencing it. But now you've added some type safety to your code, because it won't allow you to define a SetValues callback that takes something other than a MainWindow pointer (or descendent) as its first argument.

Nonetheless, callbacks are still evil. The virtual interface method that wysota described is superior. I use exactly the same method to map between pure C++ and Qt worlds where a compute-intensive process must provide progress feedback to the GUI. A virtual interface defines the how the process posts its progress; a QObject class implements this interface and turns the messages into signals. I simply pass a pointer to a "ProgressReporter" instance into my process, and the process has no idea that it is talking to a ProgressReportDialog. Even better, since the compute code lives in a library, my unfortunate colleagues who are still coding using MFC can create an MFC object with the same interface, and it works just fine for them, too.

amleto
15th September 2012, 17:38
"my unfortunate colleagues who are still coding using MFC"
hehe.

d_stranz
15th September 2012, 19:29
hehe.

Yeah, some of us are lucky and get to live the good life. Wine, women, good food, Qt. Others are still trying to remember what "pszcStrFoo" means while losing sleep and their hair.

amleto
15th September 2012, 22:56
I recently refused an interview opportunity based solely on the fact that they (still) use MFC and COM. yuk.