PDA

View Full Version : Design Issue with Qt



ustulation
2nd February 2013, 22:11
using Qt 5.0.0

The following is roughly an Observer pattern (the code is stripped to bare minimum to explain only the problem - pls don't judge the use of the design pattern by the 'looks' of the code here):


class A : public QObject
{
Q_OBJECT
public:
void registerListner(Observer *pObs);
static A* getInstance();
signals:
void sig();
};

void A::registerListner(Observer *pObs)
{
connect(this, SIGNAL(sig()), pObs, SLOT(slo));
}

////////////////////////////////////////////////////////////////

class Observer : public QObject
{
Q_OBJECT
public slots:
virtual void slo() = 0;
};

class ConcreteObserver : public Observer , public QWidget
{
Q_OBJECT
public: //re-mentioning "slots" is not necessary i suppose
virtual void slo();
};

ConcreteObserver *pCObs = new ConcreteObserver;
A::getInstance()->registerListner(pCObs);

/////////////////////////////////////////////////////////////

problem (apart from dreaded-diamond): Can't inherit multiple times from QObject - moc does not allow it. One possible solution is derive Observer from QWidget and then ConcreteObserver from Observer alone. However this is putting a constraint on ConcreteObserver. Maybe ConcreteObserver_2 needs to derive from QDialog instead etc.

How do i solve this design problem? Is there anything specific to Qt 5.0.0 Signal-Slot (in addition to earlier versions) that can solve this, or what would you suggest?

wysota
2nd February 2013, 22:19
Observer doesn't have to inherit QObject. You can remove the whole mechanism in favor of directly connecting signals and slots instead of calling registerListener().

ustulation
3rd February 2013, 05:43
Could you please elaborate? Note that classes A and Observer will be part of the library framework and ConcreteObserver and the other code would be the client GUI/non-GUI code. Observer may have hundreds of pure virtual slots and registerListner may connect all of them to other hundreds of signals. The idea is to force the users to define all of them (slots) so that when they register to class A, no connection would be wrong (which would otherwise be apparent only during the runtime as connections are not checked during compile time unless we use function pointers/functors of Qt 5 - never used the latest connection mechanism so not very sure on this one).

wysota
3rd February 2013, 11:04
You need to understand why and when we use signals and slots (this is the third time I'm writing this in the last few weeks...). The mechanism allows to implement the so called "loosely coupled objects" paradigm (http://en.wikipedia.org/wiki/Loose_coupling). In short this means we can combine objects to function together while knowing nothing about each other. This is usually done when we have three contexts -- A, B and C. A and B know nothing of each other and C knows both A and B. This lets us to connect A and B in context C. In your situation you have just two contexts -- observer and observee (you are connecting a signal from "this") and they have to know something about each other (at least the presence of specified slots in the observer). So in the end the observee has access to a list of observers (as each has to be registered with it) and knows its interface directly. Thus instead of using signals and slots (where a signal doesn't know if anything is connected to it and what is the name of the slot(s)) you can just call specific methods directly.


void Observee::registerObserver(Observer *o) {
m_observers << o;
}

void Observee::notifySlo() {
foreach(Observer *o, m_observers) o->slo();
}

Thus no need for any QObject legacy.

On the other hand, signals and slots implements the Observer/Listener pattern itself. Thus you don't need to have any "Observer" interface nor any "Observee" interface because QObject already handles that. You can just connect signals and slots directly letting the observer decide which signals to connect to and which to ignore. Thanks to that he doesn't have to implement all those "hundreds of" slots if he's just interested in one signal. If you wish to have some control over it, you can wrap the connect statement in a function like you did but allowing the caller to specify the slot:


void Observee::registerObserver(QObject *rcvr, const char *slt) {
if(!connect(this, SIGNAL(sig()), rcvr, slt)){
qWarning() << "Connection failed";
}
}

// ...

x->registerObserver(this, SLOT(mySlot()));

Of course you can remove the whole registerObserver() call whatsoever and just let the caller specify both the signal and the slot.

ustulation
3rd February 2013, 11:53
Your first code looks more like a Qt's direct connection so i guess we will have to additionally take care when object's have different thread affinities. The second one is i suppose what i'm partially looking for. I wanted to force, by design, the users registering to implement all the slots that would be connected in registerListener. In what you showed (using QObject* instead of Observer*) i think i might achieve that at runtime using assert(connect(signal <--> slot)) and if the Observer (in this case a QObject*) does not have the required slots, one of such asserts would fail. That's almost what i want but would have been nice if i could do it statically. But i suppose there isn't a straightforward way for this or you would have told me.

I even tried this:


template<typename Base>
class Observer : public Base
{
Q_OBJECT
public slots:
virtual void slo1() = 0;
virtual void slo2() = 0;
// ....etc
}; //checking if Base derives from QObject is easy - static_cast checking should do the trick

Then the user could have the flexibility of doing:
class ConcreteObserverUser : public Observer<QWidget>
{
Q_OBJECT
public slots:
virtual void slo1();
virtual void slo2();
// ...etc
};

But again i got a moc error that templates cannot be used with Q_OBJECT :( . So i guess that static checking (leading to a compile time error) will not be possible elegantly

wysota
3rd February 2013, 22:43
Why do you want to force every slot to be implemented? It seems to me like trying to be smarter than the person that is going to be using the class I design. You offer some functionality to someone and it is his choice to use as much from this functionality as she/he wants. Trying to force her/him to use all observer callbacks doesn't feel like non-intrusive "observer" pattern.

If you really really really want to check if a class implements all the slots then you can do that using QMetaObject:


void checkAllSlotsExist(QObject *obj) {
const QMetaObject *mo = obj->metaObject();
assert(mo->indexOfMethod("slo1()") >=0);
assert(mo->indexOfMethod("slo2()") >=0);
assert(mo->indexOfMethod("slo3()") >=0);
assert(mo->indexOfMethod("slo4()") >=0);
// ...
}
You can substitute asserts with returning false to have a soft-fail version. But I still think such checks are just plain wrong. If I were to use your class and saw that I had to implement 300 empty functions just to use one, I would rather connect to the signal directly instead of using your observer pattern. Which would make your Observer class totally redundant and/or obsolete.