Partial pimpl design pattern?

[jason.cipriani]

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

 

[jason.cipriani]

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

 

[jason.cipriani]

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:perator=(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

 

[Noah Roberts]

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]

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]

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]

I'd say yes.

  Should I go all the way and put


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


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

 

[Guest]

(e-mail address removed) wrote:

  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?

 

[James Kanze]

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.