Pimpl idiom and internal object collaboration without friend declaration

Question

I'm implementing several classes using the pimpl idiom and am coming across some design issues.

Firstly, I've always seen pimpl done like this

class Object
{
public:
    Visible();
    ~Visible();
 .. etc ..
private:
    class ObjectImpl *_pimpl;
};

I have several classes which use this approach and my problem is that several of these classes need access to each others implementation details but the _pimpl pointer is delcared private.

Can anyone see the downside of declaring the _pimpl public. Obviously, if it's public then someone may accidentally (or deliberately) reassign it. (I'm ignoring the fact that "private" could be #defined as "public" and grant access anyway. If you do this then you deserve what you get).

I appreciate that my design may be flawed and would welcome any comments along those lines also.

I'm really loathe to use friends and am not sure they'll even help as you can't forward declare the Object::ObjectImpl without fully defining Object.

i.e.

 ...
 private:
    class ObjectImpl *_pimpl;
    friend class OtherObject::OtherObjectImpl; // this needs a fully qualified OtherObject
};

Thx Mark.

* UPDATE - More detail **

I have two classes, one called Command, the other called Results. I have methods on Command which return a vector of Results.

Both Command and Results use the pimpl idiom. I want the interface to Results to be as small as possible.

class Command
{
public:
    void getResults( std::vector< Results > & results );
    void prepareResults( std::vector< Results > & results );
private:
    class CommandImpl *_pimpl;
};

class Results
{
public:
    class ResultsImpl;

    Results( ResultsImpl * pimpl ) :
        _pimpl( impl )
    {
    }

private
    ResultsImpl *_pimpl;
};

Now in Command::getResults(). I inject the ResultsImpl into the Results. in Command::prepareResults() I need access to the ResultsImpl.

M.

Answer 1

I doubt there is a good reason to make the implementation public: you can always expose the implementation's functionality using public methods:

class Object
{
public:
   Object();
  ~Object();

  int GetImplementationDetail();

private:
  std::unique_ptr< ObjectImpl > _pimpl;
};

int Object::GetImplementationDetail()
{
  return pimpl->GetImplementationDetail();
}

Apart from that a class should be responsible for one thing, one thing only, and should have the bare minimum of dependencies to other classes; if you think other classes should be able to access your Object's pimpl then your design is likely flawed.

edit following the author's update: although your example is still rather vague (or at least I cannot tell the full intent of it), you seem to be misinterpreting the idiom and now try to apply it to a case where it is not usefull. As others pointed out, the 'P' stands for private. Your results class doesn't have much private implementation since all of it is public. So either try to use what I mention above and do not 'inject' anything, or just get rid of the pimpl all together and use just the Result class. If your Result's class interface should be so small that it's nothing but a pointer to another class it doesn't seem to be of much use in this case.

Answer 2

Why exactly do your classes depend on details of each other? You may re-think your design. Classes should depend on abstractions.

If you really deem your design proper, nothing stops you from providing a "source-file-private" header, like:

include/
   foo.h
src/
   foo.impl.h
   foo.c
   bar.c

and then #include foo.impl.h in both foo.c and bar.c

foo.c:
    #include "foo.impl.h"
    ...

bar.c:
    #include "foo.impl.h"
    ...

But again: Generally,

Dependency Inversion Principle:

A. High Level Modules should not depend upon low level modules. Both should depend upon abstractions.

B. Abstractions should not depend upon details. Details should depend upon abstractions.

Also make very sure to check out SOLID and GRASP, especially the point about loose coupling.

Answer 3

Making the the data member _pimple public won't buy you anything as long as those other classes do not see the definition of its type ObjectImpl - which is the very thing Pimple set out to prevent.

What you can do instead is to add a private interface to your Object class which will allow befriended classes to do whatever they need to do with an Object.

Of course, the common disclaimers about friend being a tool that should be used as rarely as possible all apply.

Answer 4

It is unnecessary to qualify the friend thusly.

If you just use friend class OtherObject, then the OtherObject class may access the necessary internals.

Personally my Pimpl are just a struct (bundle of data) and I leave the methods to operate on it in the original class.