[solved] QObject and QWidget classes, request for memeber is ambiguous

T3STY · wrote on 13 Jan 2014, 18:15

I have an abstract class that inherits QObject so it can use signals and slots. Then I have a derived class from it that also inherits QWidget. The abstract class is not meant to have any GUI so I cannot inherit QWidget in the abstract class and not inherit it after in the derived class...
@
class other;
class abstractClass : protected other, public QObject;
class derivedClass : public abstractClass, public QWidget;
@
In the derived class constructor, when i call this->setParent() or other methods available in both QObject and QWidget classes I get a compiler error saying that the call is ambiguous. If I remove QObject inheritance from the abstract class I am unable to use slots and signals (more errors during compilation). Changing inheritance from public to private or protected does not have any effect on any of the classes...

How can I escape this madness?

Chris Kawa · wrote on 13 Jan 2014, 19:20

Qt doesn't support inheriting from multiple QObject derived classes.
It might seem limiting at times, but personally I came to the conclusion that often this need is a good indicator of bad design decisions.

For example, purely conceptually, a widget is something meant to have graphical representation, so it should only inherit QWidget and focus on being a graphical representation :) If it also inherits something else, it might indicate that it does more than "one thing", thus breaking the rule of single responsibility. Basically in an ideal world one class does single thing. It might vary what "single thing" means, and sometimes additional classes are required to do that thing, but they should be just means (so rather members) and not transform the object itself (by inheritance) into that mean.

It is also a rule of thumb that you should avoid inheritance if the same can be achieved by composition. It often simplifies design and cleans up interface. It also creates opportunities for compilation speedups via forward declarations.
There's a great series at "Sutter's Mill":http://herbsutter.com/gotw/, particularly GotW #7 might interest you in this case.

So in the case you mentioned it could go something along the lines of:
@
class Abstract1 {}
class Abstract2 {}
class Implementation1 : public Abstract1 {}
class Implementation2 : public Abstract2 {}
class MyWidgetyClass: public QWidget {
private:
Implementation1 impl1;
Implementation2 impl2;
}
@
Each in separate files of course.

It's an example and it all depends on a particular case, but it's a fact of life that inheritance greatly increases coupling and is one of the most overused features in OOP.

T3STY · wrote on 13 Jan 2014, 22:41

Let me get this right... I have an abstract class that represents all the things an object should do without any GUI element; it just works in the back-end. So when I implement the GUI by inheriting my abstract class, and basically, just adding the GUI widgets, you're telling me I can't do that?!!
I didn't want to share my code at first, but looks like a look at it might just give a better idea... I removed additional parts of the code I wrote, but it's still playable like this.
FILE: Filter.h
@
class filterParams{
public:
bool bypass;
bool lock;
char gain;
filterParams();
};

class Filter : protected filterParams, private QObject{
Q_OBJECT

public:
Filter(filterParams * params);

bool getBypass();
bool getLock();
char getGain();

void setBypass(bool BYPASS=FILTER_DEF_BYPASS);
void setLock(bool LOCK=FILTER_DEF_LOCK);
bool setGain(char gain=FILTER_DEF_GAIN);

signals:
void bypassChanged(void * pFilter);
void lockChanged(void * pFilter);
void gainChanged(void * pFilter);
};
@
FILE: FilterGUI.h
@#include <QSlider>
#include <QWidget>
#include <QPushButton>
#include "Filter.h"

/** Implementing the GUI over the back-end class **/
class FilterGUI : public Filter, public QWidget{
Q_OBJECT

public:
FilterGUI(filterParams * params, QWidget * parent=nullptr, Qt::WindowFlags flags=0);
~FilterGUI();

private: // PARENT:
QSlider * gainSlider; // this
QWidget * settContainer; // this
QPushButton * closeButton; // this

QPushButton * bypassButton; // this->settContainer
QPushButton * lockButton; // this->settContainer

private slots:
void onSliderChanged(int value);
void onLockClicked(bool checked);
void onBypassClicked(bool checked);

signals:
void filterClosed(FilterGUI * pFilter);
};
@
FILE: FilterGUI.cpp
@#include "FilterGUI.h"

FilterGUI::FilterGUI(filterParams * params, QWidget * parent, Qt::WindowFlags flags){
// this is one of the ambiguous members
// next ambiguous member is connect(), emit and any this->X methods
this->setParent(parent, flags);

closeButton = new QPushButton(this);
closeButton->setGeometry(130, 0, 20, 20);
this->connect(closeButton, SIGNAL(clicked()), this, SLOT(onCloseClicked()));

gainSlider = new QSlider(this);
gainSlider->setRange(FILTER_GAIN_RNG_MIN, FILTER_GAIN_RNG_MAX);
gainSlider->setSliderPosition(gain);
gainSlider->setGeometry(0, 20, 45, 110);
this->connect(gainSlider, SIGNAL(valueChanged(int)), this, SLOT(onSliderChanged(int)));

settContainer = new QWidget(this);
settContainer->setGeometry(60,20, 90, 110);

bypassButton = new QPushButton(settContainer);
bypassButton->setText("Bypass");
bypassButton->setGeometry(0, 90, 45, 20);
bypassButton->setCheckable(true);
this->connect(bypassButton, SIGNAL(clicked(bool)), this, SLOT(onBypassClicked(bool)));

lockButton = new QPushButton(settContainer);
lockButton->setText("Lock");
lockButton->setGeometry(45, 90, 45, 20);
lockButton->setCheckable(true);
this->connect(lockButton, SIGNAL(clicked(bool)), this, SLOT(onLockClicked(bool)));
}

FilterGUI::~FilterGUI(){
delete lockButton;
lockButton = nullptr;

delete bypassButton;
bypassButton = nullptr;

delete settContainer;
settContainer = nullptr;

delete gainSlider;
gainSlider = nullptr;

delete closeButton;
closeButton = nullptr;
}

/** SLOTS **/
void FilterGUI::onCloseClicked(){
this->close();

emit filterClosed(this);
}

void FilterGUI::onBypassClicked(bool checked){
setBypass(checked); // from the back-end
}

void FilterGUI::onLockClicked(bool checked){
setLock(checked); // from the back-end
}

void FilterGUI::onSliderChanged(int value){
setGain(value); // from the back-end
}
@

As you can see, the abstract class Filter just makes the moves in the back-end while the FilterGUI class only connects a few GUI widgets with the back-end code. The Filter class could be used alone both in a console and a GUI application while FilterGUI could be only used in a GUI. The Filter class inherits QObject because this is the only base widget that allows using slots and signals. I could just move all those slots and signals in the FilterGUI class, but I'd loose that functionality in the back-end, which is something I actually need. And even more, I don't want to write again the same code of Filter in the FilterGUI class since it would be just a copy-paste code.

So it might look to you like a bad design (even if it looks pretty nice to me), but anyway I need slots and signals in both Filter and FilterGUI, and FilterGUI must inherit from Filter to avoid code repetition and easy maintenance.
Any ideas?

JKSH · wrote on 14 Jan 2014, 05:43

Hi, see:

I agree with Chris. The front-end and back-end are supposed to be separate objects, so it's a bit strange to have the front-end inherit the back-end.

To achieve proper separation of concerns while minimizing the amount of code you need to write, you should use composition instead of inheritance -- I'd say "your GUI has-a filter" (not: "your GUI is-a filter"). Make the Filter object a member variable of the GUI object. You can even connect the individual widget signals directly to your filter slots. No need to implement slots in your GUI, and no need for copy-paste code.
@
this->filter = new Filter(params, this); // Remember to set QObject parent-child relationship

connect(lockButton, SIGNAL(clicked(bool)),
filter, SLOT(setLock(bool)));
@

P.S. You don't have to delete your buttons individually either. Since you gave them a parent, they will automatically be deleted when the parent is deleted.

Chris Kawa · wrote on 14 Jan 2014, 10:03

Yeah, that's what I tried to say when I mentioned overuse of inheritance. Don't get offended though, I didn't mean to preach. I see a lot of code like that in the field and with some tricks it works. One of these is making an abstract base class not a QObject and define pure virtual protected methods which you then re-declare in the derived class as signals/slots. It's just a messy way to do it.

Your Filter should have filter params, not be filter params and your ui should respond to filter signals, not be a filter.
Keep in mind what JKSH said - inheritence expresses "is a" relationship, composition "has a" and signal/slots "responds to".

P.S It's not that you don't have to delete QObjects with parents. You shouldn't! Qt will try to delete them too so it will crash (might not on your machine but it will eventually).

NicuPopescu · wrote on 14 Jan 2014, 12:07

bq. Yeah, that’s what I tried to say when I mentioned overuse of inheritance. Don’t get offended though, I didn’t mean to preach. I see a lot of code like that in the field and with some tricks it works.

I think, the trick here would be to use the base class's resolution operator:

@QWidget::setParent(parent);@

but is still there the double QObject inheritance compiler's warning and it should not be ignored

imho the GUI objects must be separated from the application data objects, as much as possible, even decoupling them completely ... this is one principle of MVC(P): they should be created separated in the application and coupled in a very controlled way by a controller class or at least provide data members and setters in gui classes to couple(encapsulate) relevant data objects, but, also mentioned above, do not transform GUI object in data object by inheritance ...

T3STY · wrote on 14 Jan 2014, 17:06

[quote author="Chris Kawa" date="1389693780"]Yeah, that's what I tried to say when I mentioned overuse of inheritance. Don't get offended though, I didn't mean to preach.[/quote]
Not getting offended at all, and sorry for my angry answer. This issue was getting on my nerves for the whole day... but I sincerely apologize for getting angry when you only tried to help me.

I understand now that I have to USE objects of the classes created before instead of inheriting from them. It seemed to me a good idea to inherit from the previous classes so I would only have 1 object doing all the things, including the back-end things together with the front-end.
I'll now recode my work and hopefully it will get better than the mess it is right now :)

Thank you all guys for your help, I really appreciate it!

JKSH · wrote on 17 Jan 2014, 00:42

You're welcome :) Happy coding!

[quote]It seemed to me a good idea to inherit from the previous classes so I would only have 1 object doing all the things, including the back-end things together with the front-end.[/quote]That's understandable; lots of people feel this way too, because it feels simple. Unfortunately, it almost always leads to unmaintainable code.

There's even a name for it: http://en.wikipedia.org/wiki/God_object

T3STY · wrote on 17 Jan 2014, 12:29

Oh my GOb... :D I never heard of it before!
Anyway, I think my intent wasn't really a GOb since this was a 'simple' object that would manage a single Filter of an equalizer application... the sole purpose was to manage the filter itself and its related GUI widgets, while the rest of the application would think at any other tasks.
Still it's good to know about it, I'll avoid such GObs as much as possible ;)