0

I have been trying to implement a Pimpl class, following the instructions on this Qt wiki page, in which the private class inherits from another private base class.

Here is a basic example:

Base class

Header

// gadget.h 
#ifndef GADGET_H
#define GADGET_H

#include <QWidget>

class GadgetPrivate;

class Gadget : public QWidget
{
    Q_OBJECT

public:
    explicit Gadget(QWidget *parent = 0);
    ~Gadget();

protected:
    Gadget(GadgetPrivate &d, QWidget *parent = 0);

    GadgetPrivate *d_ptr;

private:
    Q_DISABLE_COPY(Gadget)
    Q_DECLARE_PRIVATE(Gadget)
};

#endif // GADGET_H

Implementation

// gadget.cpp
#include "gadget.h"
#include "gadget_p.h"

Gadget::Gadget(QWidget *parent)
    : QWidget(parent),
      d_ptr(new GadgetPrivate(this))
{}

Gadget::~Gadget() {}

Gadget::Gadget(GadgetPrivate &d, QWidget *parent)
    : QWidget(parent),
      d_ptr(&d)
{}

Private class

// gadget_p.h
#ifndef GADGET_P_H
#define GADGET_P_H

#include "gadget.h"

class GadgetPrivate
{
    Q_DECLARE_PUBLIC(Gadget)

public:
    GadgetPrivate(Gadget *q);

    Gadget *q_ptr;
};

GadgetPrivate::GadgetPrivate(Gadget *q)
    : q_ptr(q)
{}

#endif // GADGET_P_H

Subclass

Header

// gizmo.h
#ifndef GIZMO_H
#define GIZMO_H

#include "gadget.h"

class GizmoPrivate;

class Gizmo : public Gadget
{
    Q_OBJECT

public:
    explicit Gizmo(QWidget *parent = 0);
    ~Gizmo();

private:
    Q_DISABLE_COPY(Gizmo)
    Q_DECLARE_PRIVATE(Gizmo)
};

#endif // GIZMO_H

Implementation

// gizmo.cpp
#include "gizmo.h"
#include "gadget_p.h"

class GizmoPrivate : public GadgetPrivate
{
    Q_DECLARE_PUBLIC(Gizmo)

public:
    GizmoPrivate(Gizmo *q);
};

GizmoPrivate::GizmoPrivate(Gizmo *q)
    : GadgetPrivate(q)
{}

Gizmo::Gizmo(QWidget *parent)
    : Gadget(*new GizmoPrivate(this), parent)
{}

Gizmo::~Gizmo() {}

I get the following error:

 In function `GadgetPrivate::GadgetPrivate(Gadget*)':
 error: multiple definition of `GadgetPrivate::GadgetPrivate(Gadget*)'

Does anyone know what I am doing wrong here?

lazerpants
  • 3
  • 5

2 Answers2

0

This constructor declaration:

Gadget(GadgetPrivate &d, QWidget *parent)

Requires a reference to GadgetPrivate, which at this point is just a forward-declared class. Switch to the parameter type to a pointer and you'll be fine:

Gadget(GadgetPrivate *d, QWidget *parent)

and:

Gadget(GadgetPrivate *d, QWidget *parent)
  : QWidget(parent), d_ptr(d)
  {}

Some notes:

  • First, use a QScopedPointer for your d_ptr. This will prevent the memory leak you have from not deleting the d_ptr. The q_ptr should remain a raw pointer.
  • This Q&A on the PIMPL idiom is fantastic and includes lots of hints about "gotcha"s and the like.
  • Marc Mutz wrote some great articles on Qt and the pimpl idiom. They are worth checking out. I linked to part 2 of the articles because it goes into more details of the internals.
Community
  • 1
  • 1
anonymous
  • 3,754
  • 2
  • 15
  • 36
  • Why doesn't `Gadget(*new GizmoPrivate(this), parent)` correspond to `Gadget(GadgetPrivate &d, QWidget *parent = 0);`? – kfsone May 16 '16 at 20:06
  • @kfsone Duh...it does. I must have looked three times for that signature and didn't see it. – anonymous May 16 '16 at 20:11
  • You are right about the memory leak. I removed the `QScopedPointer` since I wasn't sure if it had anything to do with the error. – lazerpants May 17 '16 at 06:10
  • I tried your suggestion, but I still get the same compiler error. I think the problem is that `gadget_p.h` is included from both `gizmo.cpp` and `gadget.cpp`. If I make everything inline in the private base class, it works. See my separate answer. – lazerpants May 17 '16 at 06:27
  • Separate `cpp` files including the same header should not cause such an error. Is it possible you had an inline *and* out-of-line definition? – anonymous May 17 '16 at 11:01
  • @Jon Harper No, that's not it, but I feel that I have missed something. I also realized that it works if I put the implementation code for GadgetPrivate in `gadget.cpp`. Is that a bad idea? – lazerpants May 17 '16 at 11:24
  • No, that is perfectly sound practice. It limits what's included for classes derived from `GadgetPrivate` and thus speeds compile times (which is part of the point of PIMPL). – anonymous May 17 '16 at 11:44
0

Solution #1:

Make everything in the private base class inline, i.e., changing this,

class GadgetPrivate
{
    Q_DECLARE_PUBLIC(Gadget)

public:
    GadgetPrivate(Gadget *q);

    Gadget *q_ptr;
};

GadgetPrivate::GadgetPrivate(Gadget *q)
    : q_ptr(q)
{
}

to,

class GadgetPrivate
{
    Q_DECLARE_PUBLIC(Gadget)

public:
    GadgetPrivate(Gadget *q)
        : q_ptr(q)
    {
    }

    Gadget *q_ptr;
};

made the code compile.

Solution #2:

This also seems to work:

gadget_p.h

class GadgetPrivate
{
    Q_DECLARE_PUBLIC(Gadget)

public:
    GadgetPrivate(Gadget *q);

    Gadget *q_ptr;
};

gadget.cpp

#include "gadget.h"
#include "gadget_p.h"

GadgetPrivate::GadgetPrivate(Gadget *q)
    : q_ptr(q)
{
}

Gadget::Gadget(QWidget *parent)
    : QWidget(parent),
      d_ptr(new GadgetPrivate(this))
{
}

Gadget::~Gadget()
{
}

Gadget::Gadget(GadgetPrivate *d, QWidget *parent)
    : QWidget(parent),
      d_ptr(d)
{
}

Implementation of GadgetPrivate has moved to gadget.cpp.

lazerpants
  • 3
  • 5