1

I a project of mine, written in Qt, I have a QWidget Widget that should display either a MyTreeWidget (inheriting from QTreeWidget) or a MyTableWidget (inheriting from QTableWidget)

Constraints

  • Widget shouldn't know who it is talking to. Therefore it must (??) own a class inherited by the My(Tree|Table)Widget
  • MyTreeWidget and MyTableWidget share a lot of code and I don't want to copy paste this code. So I thought of making them inherit from a MyGenericView which inherit from QAbstractItemView

The Interfaces

#include <QAbstractItemView>
#include <QTreeWidget>
#include <QTableWidget>

class MyGenericView : public QAbstractItemView
{
    Q_OBJECT
    public:
        MyGenericView();
};

class MyTreeWidget : virtual public QTreeWidget,
                     virtual public MyGenericView
{
    Q_OBJECT
    public:
        explicit MyTreeWidget(QWidget *parent = 0);
};

class MyTableWidget : public MyGenericView, public QTableWidget { ... };

class Widget : public QWidget
{
    Q_OBJECT

    public:
        explicit Widget(QWidget *parent = 0) :
            QWidget(parent)
        {
            m_genericView = new MyTreeWidget();
        }

    private:
        MyGenericView *m_genericView;
};

The Error

erreur : invalid new-expression of abstract class type 'MyTableWidget'
m_genericView = new MyTableWidget();

note:   because the following virtual functions are pure within 'MyTableWidget':
class MyTableWidget : public QTableWidget, public MyGenericView

And the same for MyTreeWidget.

So how would you correct this?

Kuba hasn't forgotten Monica
  • 95,931
  • 16
  • 151
  • 313
ochurlaud
  • 410
  • 5
  • 14
  • 1
    1. without Q_OBJECT macro, moc is not triggered on your objects, so a lot of things goes wrong after that. 2) if you add Q_OBJECT and trigger moc, then you get an error that leads to the conclusion that moc doesn't support multiple inheritance. – user3528438 Aug 20 '15 at 12:58
  • 5
    `QTableWidget` and `QTreeWidget` are already `QAbstractItemView`s. Please reconsider your class hierarchy. – vahancho Aug 20 '15 at 12:58
  • I didn't put the Q_OBJECT macro here, but in my code there is – ochurlaud Aug 20 '15 at 12:58
  • @vahancho: Would you have another way to deal with this *specific* problem ? – ochurlaud Aug 20 '15 at 13:03
  • @ochurlaud, please explain why do you need the `MyGenericView` class? – vahancho Aug 20 '15 at 13:06
  • I read this http://stackoverflow.com/questions/6843247/problems-with-qobject-multiple-inheritance-and-policy-traits-design-in-c?rq=1, which my be a reason, isn't it? – ochurlaud Aug 20 '15 at 13:06
  • 1
    Consider replacing inheritance with aggregation, so `YourTableWidget` will contain `QTableWidget` and `YourTreeWidget` will contain `QTreeWidet` – Lol4t0 Aug 20 '15 at 13:07
  • @Lol4t0 I did this before. The problem is that if I want to deal with the item selection of both classes at once, I'm trapped again. – ochurlaud Aug 20 '15 at 13:26
  • @vahancho I need (? again) the MyGenericView to deal with the QActions, to call the right function if an item is selected and so on. And my Widget shouldn't know if it is talking to a QTableView or Tree or List. – ochurlaud Aug 20 '15 at 13:32
  • 2
    **TL;DR** Virtually deriving from `QObject` (thus any `QWidget`!) leads to undefined behavior. Qt expects that all pointers to `QObject` point to a memory area that starts with `QObject`'s internals. Thus you can neither derive from `QObject` as anything but the first derived-from class (`class Foo : public Bar, QWidget` is wrong), nor virtually derive from it, since nothing guarantees that a virtually-derived-from object will be the first thing in the in-memory layout of the derived class. – Kuba hasn't forgotten Monica Aug 20 '15 at 16:13
  • This is very closely a duplicate of [this question](http://stackoverflow.com/q/17057222/1329652). Please read the answers to the other question, and decide if they are sufficient. If they aren't, please edit your question to make it more specific. Otherwise, I'll close it as a duplicate in a day or two. – Kuba hasn't forgotten Monica Aug 20 '15 at 18:32
  • @KubaOber I don't see the connection with the other question you pointed out. It seems there really complex and specific, and here (in my case) it's quite straight-forward. I wanted to share code between 2 classes that are not so much related and didn't find a right way to do it. MikeB's answer pointed me what I needed: my idea was technically not possible and I should rather use composition/delegation. However bad he might have told it. – ochurlaud Aug 20 '15 at 21:26
  • Your idea is possible and simple to do, you're just going about it the wrong way: you need to use generic programming coupled with inheritance, not virtual inheritance. What you need to do is the same as what you'd need to do to have signals and slots in a generic base class. Once you support these, you can provide any other common functionality you want - and your generic class is deriving from the underlying view, so it's literally as if you were adding the same code to two different view classes. Somehow, your problem has the same solution as the problem of "virtual" signals and slots. – Kuba hasn't forgotten Monica Aug 20 '15 at 21:37
  • @KubaOber: I'll read that again tomorrow and come back to it. What you're proposing seems right to me. I just can't see now how it goes in my project. Don't close it too soon :) – ochurlaud Aug 20 '15 at 21:41

2 Answers2

1

It seems that what you're trying to do is ill-advised. Both views that you derive from are convenience views. They hopelessly mix up the view with the model. It's OK to use them if the needs are simple and convenience is all you're after, but in your case I presume most of the shared code is related to the model side of things, not to the view. You could probably achieve what you wish by simply showing a QStandardItemModel on either a stock QTableView or a stock QTreeView, and having a class that uses the QStandardItemModel to build up your data structure.

For more details of how you could do it, if it turned out to be the right thing to do, see this answer.

Community
  • 1
  • 1
Kuba hasn't forgotten Monica
  • 95,931
  • 16
  • 151
  • 313
  • It's not quite true... The shared code is more about the user interface: doing a right click on an item, whatever class this item come from (Table,Tree,List) the behavior should be the same. 1) show the custom context menu 2) giving the selected QAction, take the selected item and do something on it (same slots too). The model thing (what do i want to show) is already made by another class and stored in a QList. I'll looked further to the answer you gave here, because it's quite long and complex. I'll certainly learn from it. – ochurlaud Aug 20 '15 at 21:33
-1

Edit : As suggested below in comments, this answer is based on faulty assumptions. Please see the better answer below.

First, you have an issue of diamond inheritance. Your error is because MyTableWidget has an undefined pure virtual member function.

Frankly though, I'm not sure why you want to use multiple inheritance at all here. If it's to save on code duplication, why can't MyTreeWidget and MyTableWidget share behavioural elements via composition instead of inheritence? Is this definitely a case of is-a vs has-a? If it's code specific to widgets that is shared but don't overlap in any way with the QTableWidget/QTreeWidget approach, just write an adaptor class that will be filled with either a Tree or Table widget.

Mike B
  • 478
  • 2
  • 11
  • 3
    I recommend do not use virtual inheritance, at least with Qt classes. – vahancho Aug 20 '15 at 13:07
  • @vahancho I entirely agree. My answer has been updated to offer an alternative - but without a bit more information about what problem OP is trying to solve, it's difficult to suggest a solution. We have the solution using multiple inheritence that causes a diamond problem, but what problem is that solution solving, and thus which aspects of this solution need to be incorporated into the better solution, and which can be ignored? – Mike B Aug 20 '15 at 13:11
  • @MikeB At first I was doing this and all was working well. I tried to change this way to be deal also with the right click, item selection and so on on the Table/TreeWidget. Maybe it's a dead end and I should come back to composition. – ochurlaud Aug 20 '15 at 13:35
  • Please tell me if you agree with my last comment. If so, I'll validate your answer and `git reset --hard` on my project :D – ochurlaud Aug 20 '15 at 13:42
  • @ochurlaud I'm afraid I'm not sure I understand your comment. Are you saying that you can't solve the right-click issue without using multiple inheritence? This surprises me. I empathise with the drive to make a clean, elegant solution to a problem, but in the end it's often better to make something simple that looks a bit clumsy, but is easily understood and solves the exact problem presented. If you can do what you need with a simple composition solution, do that - if there's a better way of handling your specific needs, it should pop up in code review (assuming you have them) – Mike B Aug 20 '15 at 13:56
  • @MikeB Maybe I can and I wasn't doing the right way. But this would be another question. :) – ochurlaud Aug 20 '15 at 13:58
  • 2
    "Virtual inheritance with QObject is not supported." (http://doc.qt.io/qt-5/moc.html) – Lol4t0 Aug 20 '15 at 14:12
  • @Lol4t0 I did not know that! You learn something new every day - thanks! – Mike B Aug 20 '15 at 14:13
  • This question *first* suggests an approach that won't ever work for technical reasons, then goes on to ask a question that belongs in comments, not an answer. Please edit it to remove both of these. I'd also like to point out that composition with a non-generic type won't help with compile-time checked signal/slots - the only ones you should be using in 2015. For that you'd need inheritance *or* composition with a generic type that is parametrized on the derived type. In case of inheritance, the generic type will have to be parametrized on and derive from a base `QObject` type, too. – Kuba hasn't forgotten Monica Aug 20 '15 at 18:35
  • @KubaOber Done! Thanks for the catch - you're right, it shouldn't stay up as it could be misleading for others scanning this page, looking for a solution to a similar problem. – Mike B Aug 21 '15 at 11:23