Asp Forum - Partial pimpl design pattern?

jason.cipriani@gmail.com

12/6/2008 9:33:00 PM

I'm writing a class where I want to keep a few details out of the
public header, but those details are only a small fraction of
everything in the class.

Normally I'd do this with pimpl or something, but I want to avoid the
inconvenience of coding and maintaining the entire pimpl abstraction.
I always get uncomfortable when I try a new design pattern though. My
question is: Is something like the example below in poor form? Is
there a better way?

So, say I originally have a class like this:

class MyClass {
public:
MyClass ();
~MyClass ();
void a ();
void b ();
void c ();
void d ();
private:
int x_;
int y_;
};

And I only want to separate part of it. So I do this (I've inlined
function definitions to keep the example brief but note that, of
course, the actual source code is split into appropriate headers and
source files):

class MyClass {
public:
MyClass () { impl_ = new MyClassPartialImpl(this); }
~MyClass () { delete impl_; }
void a ();
void b ();
void c () { impl_->c(); }
void d () { impl_->d(); }
private:
int x_;
friend MyClassPartialImpl;
MyClassPartialImpl *impl_;
};

class MyClassPartialImpl {
public:
explicit MyClassPartialImpl (MyClass *owner) : owner_(owner) { }
~MyClassPartialImpl ();
void c ();
void d ();
private:
MyClass *owner_;
int y_;
};

Here, I've moved the functions c() and d(), and the data y_, into
MyClassPartialImpl. MyClassPartialImpl still has access to the data
x_, and the functions a() and b(), via owner_.

Am I doing something sloppy? Should I go all the way and put
everything in MyClassPartialImpl (and why)? Is there a more
traditional way to do what I'm trying to do (hide some details from
the public but keep the implementation convenient)?

Sorry, if this is a vague or silly question.
Thanks,
Jason

9 Answers

jason.cipriani@gmail.com

12/6/2008 9:36:00 PM

On Dec 6, 4:33 pm, "jason.cipri...@gmail.com"
<jason.cipri...@gmail.com> wrote:
> I'm writing a class where I want to keep a few details out of the
> public header, but those details are only a small fraction of
> everything in the class.
>
> Normally I'd do this with pimpl or something, but I want to avoid the
> inconvenience of coding and maintaining the entire pimpl abstraction.
> I always get uncomfortable when I try a new design pattern though. My
> question is: Is something like the example below in poor form? Is
> there a better way?
>
> So, say I originally have a class like this:
>
> class MyClass {
> public:
> MyClass ();
> ~MyClass ();
> void a ();
> void b ();
> void c ();
> void d ();
> private:
> int x_;
> int y_;
>
> };
>
> And I only want to separate part of it. So I do this (I've inlined
> function definitions to keep the example brief but note that, of
> course, the actual source code is split into appropriate headers and
> source files):
>
> class MyClass {
> public:
> MyClass () { impl_ = new MyClassPartialImpl(this); }
> ~MyClass () { delete impl_; }
> void a ();
> void b ();
> void c () { impl_->c(); }
> void d () { impl_->d(); }
> private:
> int x_;
> friend MyClassPartialImpl;
> MyClassPartialImpl *impl_;
>
> };
>
> class MyClassPartialImpl {
> public:
> explicit MyClassPartialImpl (MyClass *owner) : owner_(owner) { }
> ~MyClassPartialImpl ();
> void c ();
> void d ();
> private:
> MyClass *owner_;
> int y_;

Stick a "friend MyClass;" here; both MyClass::a() and MyClass::b()
would have access to the "hidden" things in MyClassPartialImpl (e.g.
y_, or any private functions).

Jason

>
> };
>
> Here, I've moved the functions c() and d(), and the data y_, into
> MyClassPartialImpl. MyClassPartialImpl still has access to the data
> x_, and the functions a() and b(), via owner_.
>
> Am I doing something sloppy? Should I go all the way and put
> everything in MyClassPartialImpl (and why)? Is there a more
> traditional way to do what I'm trying to do (hide some details from
> the public but keep the implementation convenient)?
>
> Sorry, if this is a vague or silly question.
> Thanks,
> Jason

Daniel T.

12/7/2008 1:08:00 AM

In article
<559b868f-165e-407e-8001-9e01a575e8cd@v13g2000yqm.googlegroups.com>,
"jason.cipriani@gmail.com" <jason.cipriani@gmail.com> wrote:

> On Dec 6, 4:33 pm, "jason.cipri...@gmail.com"
> <jason.cipri...@gmail.com> wrote:
> > I'm writing a class where I want to keep a few details out of the
> > public header, but those details are only a small fraction of
> > everything in the class.
> >
> > Normally I'd do this with pimpl or something, but I want to avoid the
> > inconvenience of coding and maintaining the entire pimpl abstraction.
> > I always get uncomfortable when I try a new design pattern though. My
> > question is: Is something like the example below in poor form? Is
> > there a better way?
> >
> > So, say I originally have a class like this:
> >
> > class MyClass {
> > public:
> > MyClass ();
> > ~MyClass ();
> > void a ();
> > void b ();
> > void c ();
> > void d ();
> > private:
> > int x_;
> > int y_;
> >
> > };
> >
> > And I only want to separate part of it. So I do this (I've inlined
> > function definitions to keep the example brief but note that, of
> > course, the actual source code is split into appropriate headers and
> > source files):
> >
> > class MyClass {
> > public:
> > MyClass () { impl_ = new MyClassPartialImpl(this); }
> > ~MyClass () { delete impl_; }
> > void a ();
> > void b ();
> > void c () { impl_->c(); }
> > void d () { impl_->d(); }
> > private:
> > int x_;
> > friend MyClassPartialImpl;
> > MyClassPartialImpl *impl_;
> >
> > };
> >
> > class MyClassPartialImpl {
> > public:
> > explicit MyClassPartialImpl (MyClass *owner) : owner_(owner) { }
> > ~MyClassPartialImpl ();
> > void c ();
> > void d ();
> > private:
> > MyClass *owner_;
> > int y_;
>
> Stick a "friend MyClass;" here; both MyClass::a() and MyClass::b()
> would have access to the "hidden" things in MyClassPartialImpl (e.g.
> y_, or any private functions).

I wouldn't bother with friends:

class MyClass {
public:
MyClass();
~ MyClass();
MyClass(const MyClass& that); // note you need these
void operator=(MyClass that);
void c();
private:
int x;
struct Impl;
Impl* pimpl;
};

// in cpp file

struct MyClass::Impl
{
Impl():y(0) { }
int y;
};

MyClass(): x(0), pimpl(new Impl)
{
}

MyClass::~MyClass() {
delete pimpl;
}

MyClass::MyClass(const MyClass& that)
: x(that.x)
, pimpl( new Impl( *that.pimpl ) )
{
}

void MyClass::operator=(MyClass that)
{
x = that.x;
Impl* tmp = that.pimpl;
that.pimpl = pimpl;
pimpl = tmp;
}

void MyClass::c() {
pimpl->y = 4;
x = 8;
}

jason.cipriani@gmail.com

12/7/2008 4:48:00 AM

On Dec 6, 8:07 pm, "Daniel T." <danie...@earthlink.net> wrote:
> In article
> <559b868f-165e-407e-8001-9e01a575e...@v13g2000yqm.googlegroups.com>,
>
>
>
> "jason.cipri...@gmail.com" <jason.cipri...@gmail.com> wrote:
> > On Dec 6, 4:33 pm, "jason.cipri...@gmail.com"
> > <jason.cipri...@gmail.com> wrote:
> > > I'm writing a class where I want to keep a few details out of the
> > > public header, but those details are only a small fraction of
> > > everything in the class.
>
> > > Normally I'd do this with pimpl or something, but I want to avoid the
> > > inconvenience of coding and maintaining the entire pimpl abstraction.
> > > I always get uncomfortable when I try a new design pattern though. My
> > > question is: Is something like the example below in poor form? Is
> > > there a better way?
>
> > > So, say I originally have a class like this:
>
> > > class MyClass {
> > > public:
> > > MyClass ();
> > > ~MyClass ();
> > > void a ();
> > > void b ();
> > > void c ();
> > > void d ();
> > > private:
> > > int x_;
> > > int y_;
>
> > > };
>
> > > And I only want to separate part of it. So I do this (I've inlined
> > > function definitions to keep the example brief but note that, of
> > > course, the actual source code is split into appropriate headers and
> > > source files):
>
> > > class MyClass {
> > > public:
> > > MyClass () { impl_ = new MyClassPartialImpl(this); }
> > > ~MyClass () { delete impl_; }
> > > void a ();
> > > void b ();
> > > void c () { impl_->c(); }
> > > void d () { impl_->d(); }
> > > private:
> > > int x_;
> > > friend MyClassPartialImpl;
> > > MyClassPartialImpl *impl_;
>
> > > };
>
> > > class MyClassPartialImpl {
> > > public:
> > > explicit MyClassPartialImpl (MyClass *owner) : owner_(owner) { }
> > > ~MyClassPartialImpl ();
> > > void c ();
> > > void d ();
> > > private:
> > > MyClass *owner_;
> > > int y_;
>
> > Stick a "friend MyClass;" here; both MyClass::a() and MyClass::b()
> > would have access to the "hidden" things in MyClassPartialImpl (e.g.
> > y_, or any private functions).
>
> I wouldn't bother with friends:
>
> class MyClass {
> public:
> MyClass();
> ~ MyClass();
> MyClass(const MyClass& that); // note you need these
> void operator=(MyClass that);
> void c();
> private:
> int x;
> struct Impl;
> Impl* pimpl;
>
> };
>
> // in cpp file
>
> struct MyClass::Impl
> {
> Impl():y(0) { }
> int y;
>
> };
>
> MyClass(): x(0), pimpl(new Impl)
> {
>
> }
>
> MyClass::~MyClass() {
> delete pimpl;
>
> }
>
> MyClass::MyClass(const MyClass& that)
> : x(that.x)
> , pimpl( new Impl( *that.pimpl ) )
> {
>
> }
>
> void MyClass::operator=(MyClass that)
> {
> x = that.x;
> Impl* tmp = that.pimpl;
> that.pimpl = pimpl;
> pimpl = tmp;
>
> }
>
> void MyClass::c() {
> pimpl->y = 4;
> x = 8;
>
> }
>
>

I see what you mean, thanks for the tip. I like that better too. I
guess it's not actually that unusual, I called it "pimpl" but really
it's just a class with a pointer to another class as a member
variable, nothing special.

Thanks,
Jason

john

12/8/2008 4:54:00 PM

jason.cipriani@gmail.com wrote:

>
> Here, I've moved the functions c() and d(), and the data y_, into
> MyClassPartialImpl. MyClassPartialImpl still has access to the data
> x_, and the functions a() and b(), via owner_.

Eww!

>
> Am I doing something sloppy?

I'd say yes.

Should I go all the way and put
> everything in MyClassPartialImpl (and why)?

Keep business logic in one place. If you're spliting the class maybe
the class really needs to be split...publically. Otherwise put it all
in the same object. Having a pimpl that refers to the handle to do its
work just does not adhere to the spirit of the idiom and would add
unnecessary complexity. Furthermore you're not going to get the major
benefits of the handle/body, exception safety, with this design.

Is there a more
> traditional way to do what I'm trying to do (hide some details from
> the public but keep the implementation convenient)?

Put the entire implementation in the pimpl and only have a shell object
that refers to the pimpl for real behavior. If you need to inherit from
this object then provide protected member functions that are virtual to
do so...and provide protected member functions to call the pimpl where
appropriate.

Juha Nieminen

12/8/2008 8:03:00 PM

jason.cipriani@gmail.com wrote:
> I'm writing a class where I want to keep a few details out of the
> public header

One could ask why.

Unless those implementation details can be shared among many objects
of that class (eg. using reference counting or CoW), the pimpl idiom
only makes the class less efficient and consume more memory, and the
only "advantage" is more or less cosmetic.

The only situation where I would use something similar to pimpl would
be like this:

class OuterClass
{
class HugeInnerClass
{
// A huge amount of stuff here
};

HugeInnerClass hugeObject;

public:
...
};

If the inner class is very large, it makes the header also very large
for something which is only an internal implementation detail. In this
case it might be sensible to only forward-declare HugeInnerClass and
change the member variable to a pointer instead.

Even in this situation the advantage is only cosmetic. (Ok, if this
header is included a lot, it might make compiling slightly faster.)

jason.cipriani@gmail.com

12/8/2008 8:29:00 PM

On Dec 8, 3:02 pm, Juha Nieminen <nos...@thanks.invalid> wrote:
> jason.cipri...@gmail.com wrote:
> > I'm writing a class where I want to keep a few details out of the
> > public header
>
> One could ask why.
....[snip]...
> Even in this situation the advantage is only cosmetic. (Ok, if this
> header is included a lot, it might make compiling slightly faster.)

Some of the reason is cosmetic, some of it is compile time. Mostly
it's because the headers I have to include are out of my control, but
have a lot of unfortunate namespace pollution. The file itself is
included by most source files in the project (which consists of
upwards of 200 source files), so compile time is an issue as well.

> Unless those implementation details can be shared among many objects
> of that class (eg. using reference counting or CoW), the pimpl idiom
> only makes the class less efficient and consume more memory, and the
> only "advantage" is more or less cosmetic.

Another valid reason, by the way, is when providing a class through a
shared object interface; especially if member data includes things
like STL containers, or other objects that you don't want to expose
through the shared library.

I see what you are saying, though.

Thanks for the reply,
Jason

jason.cipriani@gmail.com

12/8/2008 8:31:00 PM

On Dec 8, 11:54 am, Noah Roberts <u...@example.net> wrote:
> jason.cipri...@gmail.com wrote:
>
> > Here, I've moved the functions c() and d(), and the data y_, into
> > MyClassPartialImpl. MyClassPartialImpl still has access to the data
> > x_, and the functions a() and b(), via owner_.
>
> Eww!
>
>
>
> > Am I doing something sloppy?
>
> I'd say yes.
>
> Should I go all the way and put
>
> > everything in MyClassPartialImpl (and why)?
>
> Keep business logic in one place. If you're spliting the class maybe
> the class really needs to be split...publically. Otherwise put it all
> in the same object. Having a pimpl that refers to the handle to do its
> work just does not adhere to the spirit of the idiom and would add
> unnecessary complexity. Furthermore you're not going to get the major
> benefits of the handle/body, exception safety, with this design.
>
> Is there a more
>
> > traditional way to do what I'm trying to do (hide some details from
> > the public but keep the implementation convenient)?
>
> Put the entire implementation in the pimpl and only have a shell object
> that refers to the pimpl for real behavior. If you need to inherit from
> this object then provide protected member functions that are virtual to
> do so...and provide protected member functions to call the pimpl where
> appropriate.

In the end, I couldn't convince myself that what I was doing was right
and so I did go all the way and split the entire class. It probably
ended up being more convenient anyways. I also did a small redesign
and was able to minimize the class that I had to split.

Thanks,
Jason

Nick Keighley

12/9/2008 8:42:00 AM

On 8 Dec, 20:02, Juha Nieminen <nos...@thanks.invalid> wrote:
> jason.cipri...@gmail.com wrote:

> > I'm writing a class where I want to keep a few details out of the
> > public header
>
> One could ask why.
>
> Unless those implementation details can be shared among many objects
> of that class (eg. using reference counting or CoW), the pimpl idiom
> only makes the class less efficient and consume more memory, and the
> only "advantage" is more or less cosmetic.

suppose the class is relativly volatile in its private part
and has many clients. This could have an impact on compilation
time.

> The only situation where I would use something similar to pimpl would
> be like this:
>
> class OuterClass
> {
> class HugeInnerClass
> {
> // A huge amount of stuff here
> };
>
> HugeInnerClass hugeObject;
>
> public:
> ...
>
> };
>
> If the inner class is very large, it makes the header also very large
> for something which is only an internal implementation detail. In this
> case it might be sensible to only forward-declare HugeInnerClass and
> change the member variable to a pointer instead.
>
> Even in this situation the advantage is only cosmetic. (Ok, if this
> header is included a lot, it might make compiling slightly faster.)

not a lot faster? What if there are a lot of private methods
and their signatures change?

--
Nick Keighley

James Kanze

12/9/2008 10:30:00 AM

On Dec 8, 9:02 pm, Juha Nieminen <nos...@thanks.invalid> wrote:
> jason.cipri...@gmail.com wrote:
> > I'm writing a class where I want to keep a few details out of the
> > public header

> One could ask why.

Doubtlessly to reduce coupling.

> Unless those implementation details can be shared among many
> objects of that class (eg. using reference counting or CoW),
> the pimpl idiom only makes the class less efficient and
> consume more memory, and the only "advantage" is more or less
> cosmetic.

The only advantage is that it reduces coupling. It's a trade
off: you loose a little runtime, and gain a lot in terms of
programmer productivity. For everything except the simplest
classes, the compilcation firewall idiom is "standard".

> The only situation where I would use something similar to
> pimpl would be like this:

> class OuterClass
> {
> class HugeInnerClass
> {
> // A huge amount of stuff here
> };

> HugeInnerClass hugeObject;
>
> public:
> ...
> };

> If the inner class is very large, it makes the header also
> very large for something which is only an internal
> implementation detail.

Even if there is no inner class. If your class has member
variables, their types must be fully defined, which in turn
means that you need to include the header files which defines
them. If the member variables are in a separate implementation
class, then there is no need for their types to even be
declared, much less defined. And you can add and remove member
variables without recompiling client code. You've reduced
coupling significantly.

--
James Kanze (GABI Software) email:james.kanze@gmail.com
Conseils en informatique orientée objet/
Beratung in objektorientierter Datenverarbeitung
9 place Sémard, 78210 St.-Cyr-l'École, France, +33 (0)1 30 23 00 34

comp.lang.c++

Partial pimpl design pattern?

jason.cipriani@gmail.com

jason.cipriani@gmail.com

Daniel T.

jason.cipriani@gmail.com

john

Juha Nieminen

jason.cipriani@gmail.com

jason.cipriani@gmail.com

Nick Keighley

James Kanze

x Login to ForumsZone