PIMPL for Multiple Inheritance

ModelTech

I need some help of a C++ expert on the following. I have created a data structure in which I have one very useful case of multiple inheritance. The reason for using multiple inheritance is that I do not need to duplicate a lot of code. A rough class diagram of this situation is illustrated below, where the italic classes are abstract classes. All these classes are part of a shared library and all these classes can be used in apps dynamically linked to the shared library (by exporting them conform the explanation given here.

Class Diagram

My challenge is that I would like to extend the situation as follows. I wish to ensure that any setters/modifiers of the shown classes invoke QUndoCommands. This means that apps that link to my shared library should not be able to access any setters/modifiers that bypass invocation of QUnodCommands. On the other hand, QUndoCommands require public setters/modifiers for the classes they need to change. In combination with the reasoning for using opaque pointers / d-pointers, I came to the conclusion that using the PIMPL paradigm is the way to go. However, I also need wish to keep memory usage low, and therefore, I wish to apply the optimization approach described here. So, I have managed to realize part of this story using the codes that are shown below (excluding the code related to the ProcessClass and Graph classes in the class diagram). The problem is of course that for ClusterClass, there are two inherited d_ptr variables. I would like to learn what the best solution would be in this situation. Any help is appreciated!

entity_p.h

#include "entity.h"
class DFMEntityPrivate {
public:
    explicit EntityPrivate(Entity *, const QString &);    // Constructor
    Entity *q_ptr;                                        // Q-Pointer
    QString Name;                                         // Attribute
};

entity_p.cpp

#include "entity_p.h"
EntityPrivate::EntityPrivate(Entity *q, const QString &N) : q_ptr(q), Name(N) { }    // Constructor

entity.h

#include <QString>
class EntityPrivate;                                               // Forward Declaration
class EXPORT Entity {                                              // Visible to App
public:
    virtual ~Entity();                                             // Destructor
    const QString &name();                                         // Getter
    virtual void rename(const QString &) = 0;                      // Setter to be implemented by subclasses
protected:
    explicit Entity(EntityPrivate &, const QString &Name = "");    // Constructor accessible to subclasses
    EntityPrivate *d_ptr;                                          // D-Pointer
};

entity.cpp

#include "entity_p.h"
Entity::Entity(EntityPrivate &d, const QString &N) : d_ptr(&d) { d_ptr->Name = N; }   // Constructor
Entity::~Entity() { delete d_ptr; }                                                   // Destructor
const QString &Entity::name() { return d_ptr->Name; }                                 // Getter

instantiable_p.h

#include "entity_p.h"
#include "instantiable.h"
class InstantiablePrivate : public EntityPrivate {
public:
    explicit InstantiablePrivate(Instantiable *, const QString &Name = ""); // Constructor
};

instantiable_p.cpp

#include "instantiable_p.h"
InstantiablePrivate::InstantiablePrivate(Instantiable *q, const QString &N) : EntityPrivate(q, N) { } // Constructor

instantiable.h

#include <QObject>
#include "entity.h"
class InstantiablePrivate;                                                  // Forward Declaration
class EXPORT Instantiable : public QObject, public Entity {                 // Visible to App
    Q_OBJECT
protected:
    explicit Instantiable(InstantiablePrivate &, const QString &Name = ""); // Constructor
};

instantiable.cpp

#include "instantiable_p.h"
Instantiable::Instantiable(InstantiablePrivate &d, const QString &N) : Entity(d, N) { } // Constructor

structural_p.h

#include <QList>
#include "structural.h"
#include "instance.h"
#include "link.h"
class StructuralPrivate {
public:
    explicit StructuralPrivate(Structural *); // Constructor
    Structural *q_ptr;                        // Q-Pointer
    QList<Instance *> Instances;              // Attribute
    QList<Link*> Links;                       // Attribute
};

structural_p.cpp

#include "structural_p.h"
StructuralPrivate::StructuralPrivate(Structural *q) : q_ptr(q) { } // Constructor

structural.h

class StructuralPrivate;                      // Forward Declaration
class EXPORT Structural {                     // Visible to App
public:
    virtual ~Structural();                    // Destructor
protected:
    explicit Structural(StructuralPrivate &); // Constructor accessible to subclasses
    StructuralPrivate *d_ptr;                 // D-Pointer
};

structural.cpp

#include "structural_p.h"
Structural::Structural(StructuralPrivate &d) : d_ptr(&d) { } // Constructor

clusterclass_p.h

#include "structural_p.h"
#include "instantiable_p.h"
#include "clusterclass.h"
class ClusterClassPrivate : public InstantiablePrivate, public StructuralPrivate {
public:
    explicit ClusterClassPrivate(ClusterClass *, const QString &); // Constructor
};

clusterclass_p.cpp

#include "clusterclass_p.h"
ClusterClassPrivate::ClusterClassPrivate(ClusterClass *q, const QString &N) : InstantiablePrivate(q, N), StructuralPrivate(q) { } // Constructor

clusterclass.h

#include "structural.h"
#include "instantiable.h"
class EXPORT ClusterClass : public Instantiable, public Structural { // Visible to App
    Q_OBJECT
public:
    explicit ClusterClass(const QString &Name = tr("Cluster"));      // Constructor
};

clusterclass.cpp

#include "clusterclass_p.h"
ClusterClass::ClusterClass(const QString &N) : Instantiable(*new ClusterClassPrivate(this, N), N), Structural(&d_ptr) { } // Constructor
// THIS IS ERRORNOUS AS d_ptr EXISTS IN MULTPLE INHERITED CLASSES

VRonin

d_ptr should be be accessed only via the d_func and Class(ClassPrivate&) constructor.
You can avoid shadowing just by changing the name of the second pimpl member to anything else (m_dptr for example). You won't be able to use the convenience of the Q_D and Q_DECLARE_P* macros but you can always just copy paste the macro body and just change the name

kshegunov

Virtual inheritance of QObject isn't allowed, and you didn't do it as you should've. Something more, it is a bad idea in the general case. My advice is to rethink your hierarchy.

ModelTech

Thanks for the interesting and useful replies!

@VRonin said in PIMPL for Multiple Inheritance:
I have now used a separate pimpl class for each api class in my architecture, where each of my classes has a reference to its pimpl class (and each pimpl class has a reference back to its api class if needed). There is no hierarchy between the pimpl classes anymore and I don't use the memory optimization trick. This has now solved my problem although perhaps not the best in style (due to requiring non-standard names, I don't use any d-func or alike macros). Moreover, my analyzes have shown that memory usage increases only an acceptable little bit. So, I am happy with my solution, which I have included at the end of this post.

@kshegunov said in PIMPL for Multiple Inheritance:

Virtual inheritance of QObject isn't allowed,

I presume with "virtual inheritance of QObject" you mean that ClusterClass inherits from QObject but it is an abstract class. Is that correct? Could you please explain your concern: why is "virtual inheritance of QObject" not allowed?

and you didn't do it as you should've.

I am not sure here what "it" refers to in this sentence: does it refer to "multiple inheritance" or to "virtual inheritance of QObject"?

If "it" refers here to "virtual inheritance of QObject", I read this remark as if "virtual inheritance of QObject" is allowed but I did not do it in the advised way. If so, what is the advised way?

If "it" refers here to "multiple inheritance", then I am aware of the potential issues it may bring. Essential for me is however to have very lean & mean interfaces (for usability reasons) without too much code duplication (for maintenance reasons). I actually have some other cases of "multiple inheritance" in my architecture and it all works perfectly fine as I envisioned it.

Something more, it is a bad idea in the general case.

I presume that "it" in this sentence does refer to "multiple inheritance".

To be complete, my solution looks as follows (note how I renamed the classes with a prefix to ease knowing what I am doing):

entity_p.h

#include "entity.h"
class DFMEntityPrivate {
public:
    explicit DFMEntityPrivate(const QString &);    // Constructor
    QString Name;                                  // Attribute
};

entity_p.cpp

#include "entity_p.h"
DFMEntityPrivate::DFMEntityPrivate(const QString &N) : Name(N) { }    // Constructor

entity.h

#include <QString>
class DFMEntityPrivate;                                     // Forward Declaration
class EXPORT DFMEntity {                                    // Visible to App
public:
    virtual ~DFMEntity();                                   // Destructor
    const QString &name();                                  // Getter
protected:
    explicit DFMEntity(const QString &Name = QString());    // Constructor accessible to subclasses
    DFMEntityPrivate *Entity;                               // D-Pointer
};

entity.cpp

#include "entity_p.h"
DFMEntity::DFMEntity(const QString &N) : EntityPrivate(new DFMEntityPrivate(N) { }   // Constructor
DFMEntity::~DFMEntity() { }                                                          // Destructor
const QString &DFMEntity::name() { return EntityPrivate->Name; }                     // Getter

instantiable_p.h

#include <QList>
#include "instantiable.h"
#include "port.h"
class DFMInstantiablePrivate {
public:
    explicit DFMInstantiablePrivate(DFMInstantiable *); // Constructor
    DFMInstantiable *Instantiable;                      // Q-Pointer
    QList<DFMPort*> Ports;                              // Attribute
};

instantiable_p.cpp

#include "instantiable_p.h"
DFMInstantiablePrivate::DFMInstantiablePrivate(DFMInstantiable *I) : Instantiable(I) {
    Ports = QList<DFMPort *>();
} // Constructor

instantiable.h

#include <QObject>
#include "entity.h"
class DFMInstantiablePrivate;                                        // Forward Declaration
class EXPORT DFMInstantiable : public QObject, public DFMEntity {    // Visible to App
    Q_OBJECT
public:
    virtual void rename(const QString &) = 0;                        // To be implemented by subclasses as they have specific aspects
protected:
    explicit DFMInstantiable(const QString &Name = QString());       // Constructor
};

instantiable.cpp

#include "instantiable_p.h"
#include "entity_p.h"
DFMInstantiable::DFMInstantiable(const QString &N) : QObject(), DFMEntity(N), InstantiablePrivate(new DFMInstantiablePrivate(this)) { } // Constructor

structural_p.h

#include <QList>
#include "structural.h"
#include "instance.h"
#include "link.h"
class DFMStructuralPrivate {
public:
    explicit DFMStructuralPrivate(DFMStructural *); // Constructor
    DFMStructural *Structural;                      // Q-Pointer
    QList<Instance *> Instances;                    // Attribute
    QList<Link*> Links;                             // Attribute
};

structural_p.cpp

#include "structural_p.h"
DFMStructuralPrivate::DFMStructuralPrivate(DFMStructural *S) : Structural(S) {
    Instances = QList<DFMInstance*>();
    Links = QList<DFMLink*>();
} // Constructor

structural.h

class DFMStructuralPrivate;                      // Forward Declaration
class EXPORT DFMStructural {                     // Visible to App
public:
    virtual ~DFMStructural();                    // Destructor
protected:
    explicit DFMStructural();                    // Constructor accessible to subclasses
    DFMStructuralPrivate *StructuralPrivate;     // D-Pointer
};

structural.cpp

#include "structural_p.h"
DFMStructural::DFMStructural() : StructuralPrivate(new DFMStructuralPrivate(this)) { } // Constructor

clusterclass_p.h

#include "structural_p.h"
#include "instantiable_p.h"
#include "clusterclass.h"
class DFMClusterClassPrivate {
public:
    explicit DFMClusterClassPrivate(ClusterClass *); // Constructor
    DFMClusterClass *ClusterClass                    // Q-Pointer
};

clusterclass_p.cpp

#include "clusterclass_p.h"
DFMClusterClassPrivate::DFMClusterClassPrivate(DFMClusterClass *C) : ClusterClass(C) { } // Constructor

clusterclass.h

#include "structural.h"
#include "instantiable.h"
class DFMClusterClassPrivate;                                                 // Forward Declaration
class EXPORT DFMClusterClass : public DFMInstantiable, public DFMStructural { // Visible to App
    Q_OBJECT
public:
    explicit DFMClusterClass(const QString &Name = tr("Cluster"));            // Constructor
    void rename(const QString&) Q_DECL_OVERRIDE;                              // Modifier
private:
    DFMClusterClassPrivate *ClusterClassPrivate;                              // D-Pointer
};

clusterclass.cpp

#include "clusterclass_p.h"
DFMClusterClass::DFMClusterClass(const QString &N) : DFMInstantiable(N), DFMStructural(), ClusteClassPrivate(new DFMClusterClassPrivate(this)) { } // Constructor
void DFMClusterClass::rename(const QString &N) { ... EntityPrivate->Name = N; ... } // Modifier

kshegunov

@ModelTech said in PIMPL for Multiple Inheritance:

@kshegunov said in PIMPL for Multiple Inheritance:

Virtual inheritance of QObject isn't allowed,

I presume with "virtual inheritance of QObject" you mean that ClusterClass inherits from QObject but it is an abstract class. Is that correct?

No, I meant virtual inheritance. But it's my bad, I thought your ClusterClass inherits from QObject (indirectly) through Graph, thus forming a diamond (along with it inheriting QObject through Instantiable), however it seems I was wrong as I look at the diagram again.

Could you please explain your concern: why is "virtual inheritance of QObject" not allowed?

It simply isn't supported for QObject and its descendants, it's not a concern but a statement of fact. The reason is that QObject has internal structure which doesn't allow for virtual inheritance.

and you didn't do it as you should've.

I am not sure here what "it" refers to in this sentence: does it refer to "multiple inheritance" or to "virtual inheritance of QObject"?

If "it" refers here to "virtual inheritance of QObject", I read this remark as if "virtual inheritance of QObject" is allowed but I did not do it in the advised way. If so, what is the advised way?

If "it" refers here to "multiple inheritance", then I am aware of the potential issues it may bring. Essential for me is however to have very lean & mean interfaces (for usability reasons) without too much code duplication (for maintenance reasons). I actually have some other cases of "multiple inheritance" in my architecture and it all works perfectly fine as I envisioned it.

Something more, it is a bad idea in the general case.

I presume that "it" in this sentence does refer to "multiple inheritance".

"It" refers to virtual inheritance, which is a special case of multiple inheritance, however my point is moot due to my misreading the original diagram.

ModelTech

@kshegunov Thanks for your clarification! Very much appreciated. Indeed, I avoid the diamond situation in any case and therefore also for QObject. I have just used doxygen to generate a class diagram (inheritance graph) for my architecture at the moment.

Inheritance Graph