Help converting standard class to use d-pointers

[jiveaxe]

Hi, today I read a blog post about d-pointers and private class:

http://zchydem.enume.net/2010/01/19/qt-howto-private-classes-and-d-pointers/

I decided to experiment with it. I started creating a standard class, MyWidget:

mywidget.h


#ifndef MYWIDGET_H
#define MYWIDGET_H

#include <QWidget>

namespace Ui {
class MyWidget;
}

class MyWidget : public QWidget
{
Q_OBJECT

public:
explicit MyWidget(const QString &first,
const QString &second,
QWidget *parent = 0);
~MyWidget();

QString first() const;
void setFirst(const QString);

QString second() const;
void setSecond(const QString);

private:
Ui::MyWidget *ui;

QString _first;
QString _second;
};

#endif // MYWIDGET_H


mywidget.cpp

#include "mywidget.h"
#include "ui_mywidget.h"

MyWidget::MyWidget(const QString &first, const QString &second, QWidget *parent) :
QWidget(parent),
ui(new Ui::MyWidget),
_first(first),
_second(second)
{
ui->setupUi(this);
}

MyWidget::~MyWidget()
{
delete ui;
}

QString MyWidget::first() const
{
return _first;
}

void MyWidget::setFirst(const QString text)
{
_first = text;
}

QString MyWidget::second() const
{
return _second;
}

void MyWidget::setSecond(const QString text)
{
_second = text;
}

then I tryed to implement private class/d-pointers; here it is the final work:

mywidget_p.h


#ifndef MYWIDGET_P_H
#define MYWIDGET_P_H

#include <QWidget>

#include "mywidget.h"

class MyWidgetPrivate : public MyWidget
{
Q_DECLARE_PUBLIC(MyWidget)

public:
MyWidgetPrivate() {}

QString first;
QString second;
};

#endif // MYWIDGET_P_H


mywidget.h


#ifndef MYWIDGET_H
#define MYWIDGET_H

#include <QWidget>

namespace Ui {
class MyWidget;
}

class MyWidgetPrivate;

class MyWidget : public QWidget
{
Q_OBJECT
Q_PROPERTY(QString first READ first WRITE setFirst)
Q_PROPERTY(QString second READ second WRITE setSecond)

public:
explicit MyWidget(const QString &first,
const QString &second,
QWidget *parent = 0);
~MyWidget();

QString first() const;
void setFirst(const QString);
QString second() const;
void setSecond(const QString);

protected:
MyWidgetPrivate * const d;

private:
Ui::MyWidget *ui;
Q_DECLARE_PRIVATE(MyWidget)
};

#endif // MYWIDGET_H


mywidget.cpp


#include "mywidget_p.h"
#include "ui_mywidget.h"

MyWidget::MyWidget(const QString &first, const QString &second, QWidget *parent) :
QWidget(parent),
ui(new Ui::MyWidget),
first(first),
second(second)
{
ui->setupUi(this);
}

MyWidget::~MyWidget()
{
delete ui;
}

QString MyWidget::first() const
{
return d->first;
}

void MyWidget::setFirst(const QString text)
{
Q_D(MyWidget);
d->first = text;
}

QString MyWidget::second() const
{
return d->second;
}

void MyWidget::setSecond(const QString text)
{
Q_D(MyWidget);
d->second = text;
}


compiling, I got


In file included from mywidget.cpp:1:0:
mywidget_p.h: In member function ‘MyWidget* MyWidgetPrivate::q_func()’:
mywidget_p.h:10:5: error: ‘q_ptr’ was not declared in this scope
mywidget_p.h: In member function ‘const MyWidget* MyWidgetPrivate::q_func() const’:
mywidget_p.h:10:5: error: ‘q_ptr’ was not declared in this scope
mywidget_p.h: In constructor ‘MyWidgetPrivate::MyWidgetPrivate()’:
mywidget_p.h:13:23: error: no matching function for call to ‘MyWidget::MyWidget()’
mywidget_p.h:13:23: note: candidates are:
In file included from mywidget_p.h:6:0,
from mywidget.cpp:1:
mywidget.h:19:14: note: MyWidget::MyWidget(const QString&, const QString&, QWidget*)
mywidget.h:19:14: note: candidate expects 3 arguments, 0 provided
mywidget.h:12:7: note: MyWidget::MyWidget(const MyWidget&)
mywidget.h:12:7: note: candidate expects 1 argument, 0 provided
mywidget.cpp: In constructor ‘MyWidget::MyWidget(const QString&, const QString&, QWidget*)’:
mywidget.cpp:7:5: error: class ‘MyWidget’ does not have any field named ‘first’
mywidget.cpp:8:5: error: class ‘MyWidget’ does not have any field named ‘second’
mywidget.cpp:4:1: error: uninitialized member ‘MyWidget::d’ with ‘const’ type ‘MyWidgetPrivate* const’ [-fpermissive]
make: *** [mywidget.o] Errore 1


Where am I wrong?

Thanks.

[anda_skoa]

The private does not inherit the main class, it is a totally independent one.

You will see this better if you don't use the Qt macros while you are still experimenting with the idea, i.e. write code manually.




class MyWidget : public QWidget
{
Q_OBJECT
Q_PROPERTY(QString first READ first WRITE setFirst)
Q_PROPERTY(QString second READ second WRITE setSecond)

public:
explicit MyWidget(const QString &first,
const QString &second,
QWidget *parent = 0);
~MyWidget();

QString first() const;
void setFirst(const QString);
QString second() const;
void setSecond(const QString);

private:
class MyWidgetPrivate;
MyWidgetPrivate * const d;
};

The only member left is the d pointter.

All members go into the private


class MyWidgetPrivate
{
MyWidget * const q; // not stricly necessary, just in case you put methods into MyWidgetPrivate that need access to MyWidget

public:
explicit MyWidgetPrivate( MyWidget *parent ) : q( parent ), ui( new Ui::MyWidget )
{
ui->setupUi( parent );
}

Ui::MyWidget *ui;

QString _first;
QString _second;
};




MyWidget::MyWidget(const QString &first, const QString &second, QWidget *parent) :
QWidget(parent),
d( new MyWidgetPrivate( this ) )
{
}

MyWidget::~MyWidget()
{
delete d;
}


Cheers,
_

[jiveaxe]

First of all, thanks for your help. I tryed editing the code following your guide lines.

This is new mywidget_p.h



#ifndef MYWIDGET_P_H
#define MYWIDGET_P_H

#include <QWidget>

#include "mywidget.h"

class MyWidgetPrivate
{
MyWidget * const q; // not stricly necessary, just in case you put methods into MyWidgetPrivate that need access to MyWidget

public:
explicit MyWidgetPrivate( MyWidget *parent ) : q( parent ), ui( new Ui::MyWidget )
{
ui->setupUi( parent );
}

Ui::MyWidget *ui;

QString first;
QString second;
};

#endif // MYWIDGET_P_H


and this is mywidget.h


#ifndef MYWIDGET_H
#define MYWIDGET_H

#include <QWidget>

namespace Ui {
class MyWidget;
}

class MyWidgetPrivate;

class MyWidget : public QWidget
{
Q_OBJECT
Q_PROPERTY(QString first READ first WRITE setFirst)
Q_PROPERTY(QString second READ second WRITE setSecond)

public:
explicit MyWidget(const QString &first,
const QString &second,
QWidget *parent = 0);
~MyWidget();

QString first() const;
void setFirst(const QString);
QString second() const;
void setSecond(const QString);

private:
MyWidgetPrivate * const d;
};

#endif // MYWIDGET_H


Unfortunately the compiler fails


In file included from ../mywidget.cpp:1:0:
../mywidget_p.h: In constructor 'MyWidgetPrivate::MyWidgetPrivate(MyWidget*)':
../mywidget_p.h:13:77: error: invalid use of incomplete type 'class Ui::MyWidget'
In file included from ../mywidget_p.h:6:0,
from ../mywidget.cpp:1:
../mywidget.h:7:7: error: forward declaration of 'class Ui::MyWidget'
In file included from ../mywidget.cpp:1:0:
../mywidget_p.h:15:15: error: invalid use of incomplete type 'class Ui::MyWidget'
In file included from ../mywidget_p.h:6:0,
from ../mywidget.cpp:1:
../mywidget.h:7:7: error: forward declaration of 'class Ui::MyWidget'
../mywidget.cpp: At global scope:
../mywidget.cpp:4:1: warning: unused parameter 'first' [-Wunused-parameter]
../mywidget.cpp:4:1: warning: unused parameter 'second' [-Wunused-parameter]
In file included from /usr/include/QtGui/QWidget:1:0,
from ../mywidget_p.h:4,
from ../mywidget.cpp:1:
/usr/include/QtGui/qwidget.h: In member function 'QString MyWidget::first() const':
/usr/include/QtGui/qwidget.h:150:5: error: 'const QWidgetPrivate* QWidget::d_func() const' is private
../mywidget.cpp:17:5: error: within this context
../mywidget.cpp:17:5: error: cannot convert 'const QWidgetPrivate*' to 'const MyWidgetPrivate* const' in initialization
In file included from /usr/include/QtGui/QWidget:1:0,
from ../mywidget_p.h:4,
from ../mywidget.cpp:1:
/usr/include/QtGui/qwidget.h: In member function 'void MyWidget::setFirst(QString)':
/usr/include/QtGui/qwidget.h:150:5: error: 'QWidgetPrivate* QWidget::d_func()' is private
../mywidget.cpp:23:5: error: within this context
../mywidget.cpp:23:5: error: cannot convert 'QWidgetPrivate*' to 'MyWidgetPrivate* const' in initialization
In file included from /usr/include/QtGui/QWidget:1:0,
from ../mywidget_p.h:4,
from ../mywidget.cpp:1:
/usr/include/QtGui/qwidget.h: In member function 'QString MyWidget::second() const':
/usr/include/QtGui/qwidget.h:150:5: error: 'const QWidgetPrivate* QWidget::d_func() const' is private
../mywidget.cpp:29:5: error: within this context
../mywidget.cpp:29:5: error: cannot convert 'const QWidgetPrivate*' to 'const MyWidgetPrivate* const' in initialization
In file included from /usr/include/QtGui/QWidget:1:0,
from ../mywidget_p.h:4,
from ../mywidget.cpp:1:
/usr/include/QtGui/qwidget.h: In member function 'void MyWidget::setSecond(QString)':
/usr/include/QtGui/qwidget.h:150:5: error: 'QWidgetPrivate* QWidget::d_func()' is private
../mywidget.cpp:35:5: error: within this context
../mywidget.cpp:35:5: error: cannot convert 'QWidgetPrivate*' to 'MyWidgetPrivate* const' in initialization

[anda_skoa]

One error is that you have not included ui_widget.h in mywidget_p.h yet, so the compiler doesn't know about Ui::MyWidget
Another couple of errors suggest that you have still some Qt macro usages in your widget.cpp, most likely some Q_D you forgot to remove.

Cheers,
_

[jiveaxe]

Thank you anda_skoam (http://www.qtcentre.org/members/305-anda_skoa), your last tips solved all problems.

I have one more question: d-pointers should be limited to libraries or is good habit to use them in normal apps, too?

Regards.

P.S.
Here it is final code, if useful to others:

mywidget_p.h

#ifndef MYWIDGET_P_H
#define MYWIDGET_P_H

#include "mywidget.h"
#include "ui_mywidget.h"

namespace Ui {
class MyWidget;
}

class MyWidgetPrivate
{
MyWidget * const q; // not stricly necessary, just in case you put methods into MyWidgetPrivate that need access to MyWidget

public:
explicit MyWidgetPrivate( MyWidget *parent ) :
q( parent ), ui( new Ui::MyWidget )
{
ui->setupUi( parent );
}

Ui::MyWidget *ui;

QString first;
QString second;
};

#endif // MYWIDGET_P_H


mywidget.h

#ifndef MYWIDGET_H
#define MYWIDGET_H

#include <QWidget>

class MyWidgetPrivate;

class MyWidget : public QWidget
{
Q_OBJECT
Q_PROPERTY(QString first READ first WRITE setFirst)
Q_PROPERTY(QString second READ second WRITE setSecond)

public:
explicit MyWidget(const QString &first,
const QString &second,
QWidget *parent = 0);
~MyWidget();

QString first() const;
void setFirst(const QString);
QString second() const;
void setSecond(const QString);

private:
MyWidgetPrivate * const d;
};

#endif // MYWIDGET_H


mywidget.cpp

#include "mywidget_p.h"

MyWidget::MyWidget(const QString &first, const QString &second, QWidget *parent) :
QWidget(parent),
d( new MyWidgetPrivate( this ) )
{
d->first = first;
d->second = second;
}

MyWidget::~MyWidget()
{
delete d;
}

QString MyWidget::first() const
{
return d->first;
}

void MyWidget::setFirst(const QString text)
{
d->first = text;
}

QString MyWidget::second() const
{
return d->second;
}

void MyWidget::setSecond(const QString text)
{
d->second = text;
}

[anda_skoa]

Thank you anda_skoam (http://www.qtcentre.org/members/305-anda_skoa), your last tips solved all problems.

Excellent :)

Just in case: I often use this form of d-pointer implementation myself, but one has to understand that it has one disadvantage of the slighty more complex approach taken by Qt's macros: this simple form of a d-pointer makes all members effectively "non-const", e.f. you could write to d->second in MyWidget::second() const without the compiler saying anything.

This can of course be handy, but if you want to have compiler errors when you try to change members in a const method, then the approach used by Qt's macros can fix that.
(you don't necessarily have to use the macros themselves of course).

This alternative approach has a d-pointer member (usually called something else than "d", e.g. "d_ptr") and two accessor functions (often called "d_func()") where one is const and one non-const and the const one returning "const PrivateClass *".
The main class then does not access d-pointer directly but through the two functions. Due to const-ness of methods, the compiler will chose the const-correct d_func and thus return a const-correct pointer to the private.

In Qt's case, the Q_D macro hides that d_func access, i.e. it defines a local variable "d" that is initialized with the return of the appropriate d_func().

But again, the simple approach works as well, it is just important to know about the const "removal" :)



I have one more question: d-pointers should be limited to libraries or is good habit to use them in normal apps, too?


It depends. Not all classes in libraries need it and it can be helpful in application code as well.

What a d-pointer approach does is that it really enforces the "information hiding" feature of object oriented programming by hiding every internal thing inside the private class.

Classes in libraries that are public API use this to ensure that their memory layout stays unchanged even if data members have to be added, removed or their types changed.
The "outward" facing memory layout is always just a single pointer. A library internal class does not strictly need this, so it no code using the library will ever see it anyway.

In application code it is usually also not required to keep stable binary interfaces since the application is always relinked as one, however, d-pointer based classes can even be useful there.
One example is classes with lots of members of different complex types, i.e. if the class header would need a lot of includes or those includes in turn dragging in lots of includes.
A d-pointer approach can serve as dependency shield in this case. All code using the class only has to include that single header, any change to member types wil not trigger rebuilds of code using the class.

Cheers,
_

[jiveaxe]

Thanks for these invaluable explanations. With these new knowledge I was able to edit the code and use Qt macros:

mywidget_p.h


#ifndef MYWIDGET_P_H
#define MYWIDGET_P_H

#include "mywidget.h"
#include "ui_mywidget.h"

class MyWidgetPrivate
{
public:
explicit MyWidgetPrivate(QWidget *parent) :
ui( new Ui::MyWidget )
{
ui->setupUi( parent );
}
virtual ~MyWidgetPrivate() { delete ui; }

QString first;

Ui::MyWidget *ui;
};

#endif // MYWIDGET_P_H

mywidget.h

#ifndef MYWIDGET_H
#define MYWIDGET_H

#include <QWidget>

namespace Ui {
class MyWidget;
}

class MyWidgetPrivate;

class MyWidget : public QWidget
{
Q_OBJECT
Q_PROPERTY(QString first READ first WRITE setFirst)

public:
explicit MyWidget(const QString &text, QWidget *parent = 0);
virtual ~MyWidget();

void setFirst(QString text);
QString first() const;

protected:
MyWidgetPrivate * const d_ptr;
MyWidget(MyWidgetPrivate &dd, QWidget *parent);

private:
Q_DECLARE_PRIVATE(MyWidget);
};

#endif // MYWIDGET_H

mywidget.cpp

#include "mywidget_p.h"
#include "ui_mywidget.h"

MyWidget::MyWidget(const QString &text, QWidget *parent) :
d_ptr(new MyWidgetPrivate(parent))
{
d_ptr->first = text;
}

MyWidget::MyWidget(MyWidgetPrivate &dd, QWidget *parent)
: QWidget(parent),
d_ptr(&dd)
{
}

MyWidget::~MyWidget()
{
delete d_ptr;
}

void MyWidget::setFirst(QString text)
{
Q_D(MyWidget);
d->first = text;
}

QString MyWidget::first() const
{
Q_D(const MyWidget);
return d->first;
}