Signal/Slot best practice with composition
-
I'm developing a communication class, which is designed to be agnostic to the IPC employed. The constructor looks like this:
TapDisplayCommunicationHandler::TapDisplayCommunicationHandler( AbstractCommunicationHandler* ipc_handler_in) : QObject(), ipc_handler(ipc_handler_in), factory(), header (25,0), is_compressed_message(false), heartbeat_timer() { // this is necessary to ensure proper cleanup and to make sure this object // can be moved to another thread along with its parent, if desired ipc_handler->setParent(this); QObject::connect(this, SIGNAL(newMessage(const QByteArray&)), this, SLOT(handleNewMessage(const QByteArray&))); heartbeat_timer.setParent(this); }
The main app class which creates the instance of TapDisplayCommunicationHandler knows the IPC mechanism to be used, which has a common interface. The common interface of the IPC handler class defines signals such as:
/// signal indicating loss of connection void connectionLost(const QString&); /// signal indicating successful connection void connectionEstablished(); /// signal indicating that the system failed to connect to hardware (either initial or reactivate) void unableToConnect(); [...]
My question is this. Since the main app class creates the IPC handler instance, which is passed to the TapDisplayCommunicationHandler and integrated as a class member, is it acceptable to connect the signals from the IPC handler class to the main class directly, before passing it to the TapCommunicationHandler, or is it a better practice to have slots in TapCommunicationHandler to receive the signals from the IPC handler and then re-emit them to the main class? In the latter case, the main class would define connections with the TapCommunicationHandler instance instead of the IPC Handler instance
-
Hi,
What does TapDisplayCommunicationHandler do with AbstractCommunicationHandler ?
-
Hello @SGaist ,
In the current implementation it only implements a slot to handle the newMessage() signal from the abstract handler to decode the received QByteArray into a proper message and emitting it to the main class. It also implements a slot to handle serialization from message classes to QByteArray and emitting to the abstract handler for dispatch
-
Hi,
Perhaps I'm missing something, but why not just delegate the signals themselves. I.e. define a set of signals in the exposed (TapDisplayCommunicationHandler
) class and connect the signals fromAbstractCommunicationHandler
to the signals ofTapDisplayCommunicationHandler
. Additionally, if you proceed like this, you can insert a middleman of a slot later (if needed) that can do some stuff before reemiting the signal, but this wouldn't change your exposed class' interface (i.e. the rest of the program wouldn't care). -
@kshegunov said in Signal/Slot best practice with composition:
Hi,
why not just delegate the signals themselves. I.e. define a set of signals in the exposed (TapDisplayCommunicationHandler
) class and connect the signals fromAbstractCommunicationHandler
to the signals ofTapDisplayCommunicationHandler
.I guess that's the crux of my question. Your comment about the middleman slot is a good one. My follow-up question is what do you mean about connecting signals?
-
@DRoscoe said in Signal/Slot best practice with composition:
My follow-up question is what do you mean about connecting signals?
I mean just connecting one signal to another signal. I don't believe that's written clearly enough in the docs, so many people are left with the impression you can only connect signals to slots. Here's an example (with aggregation and delegation of signals):
class A : public QObject { Q_OBJECT signals: void someSignal(); }; class B : public QObject { Q_OBJECT public: B() { QObject::connect(&a, &A::someSignal, this, &B::someSignal); // We just connect one signal to the other - i.e. delegating } signals: void someSignal(); private: A a; };
I hope that helps. As a side note I recommend you convert to the Qt5 style of connecting (i.e. with method pointers).
Kind regards.
-
It even has a name:
signal forwarding
but indeed it looks like it's missing from the documentation
-
Glad to be of assistance.
@SGaist said in Signal/Slot best practice with composition:
it looks like it's missing from the documentation
I'm pretty sure it's written somewhere (I myself know it from reading the docs), but somehow it's mentioned only in passing and feels it's swept under the rug, which in turn leaves the ever-so-lasting doubt if it's a good practice, bad practice, allowed or discouraged.