Hide implementation details via internal pointer

Question

I have following third-party class(just wrapper around some pointer):

// a.h
class AImpl;
class A
{
    AImpl*   pImpl;

public:
    A(): pImpl(0) {}
    A(AImpl* ptr): pImpl(ptr) {}
    ...
    AImpl* getImpl() const { return pImpl; }
    ...
    void someMethodOfA();
};

I want to modify A's interface: disable some methods, add some new, while hiding its implementation details. I decided to do following:

// new_a.h
class AImpl;
class A;

class NewA
{
    AImpl*   pImpl;

public:
    NewA( const A& a );
    ...
    void newMethodOfA();
    ...
};

//new_a.cpp
#include "a.h"
NewA::NewA( const A& a ): pImpl( a.getImpl() ) {}
...
void NewA::newMethodOfA()
{
    A( pImpl ).someMethodOfA();
}
...

Is it ok to do so? Maybe there is better solution? I want to change A interface because it doesn't fit my needs and don't want to keep it in my own code.

It is important here if "A" has got virtual member functions, in that case you can derive from A and create your own set of member function, and if necessary call some from Base class. — CyberGuy, Jul 03 '13 at 18:48
@CyberGuy No, `A` has no virtual functions. It wasn't designed to be polymorphic. — user2547823, Jul 03 '13 at 18:50
What implementation are you trying to hide?. Your `pImpl` is effectively `public`. — indeterminately sequenced, Jul 03 '13 at 18:57
@indeterminatelysequenced I am trying to hide `A` implementation. `NewA::pImpl` is private by default. I don't want to allocate `A` and hold `A* pImpl`, because it's already a wrapper around some pointer (`AImpl`) — user2547823, Jul 03 '13 at 19:02
Then your `class A` is extra and unneeded. Get rid of it & add `someMethodOfA()` to `NewA`. Why would you do `pImpl( a.getImpl() )` followed by `A( pImpl ).someMethodOfA();` ! — indeterminately sequenced, Jul 03 '13 at 19:05
As a suggestion, it seems that you wanna do the same as proposed by adapter pattern: http://en.wikipedia.org/wiki/Adapter_pattern — Amadeus, Jul 03 '13 at 19:08
@indeterminatelysequenced `A` already implements pimpl idiom. I want to provide modified interface of `A` so users of `NewA` won't know anything about `A` and `AImpl` ( it will be forward declared ). I create temporary `A` object in implementation of `NewA`, using internal pointer of `A`. — user2547823, Jul 03 '13 at 19:14
Putting PIMPL on top of PIMPL? Instead I suggest you just smile and go with SIMPL — Captain Obvlious, Jul 03 '13 at 19:18
You could just hold a value of type A. Which would be an implementation of the adapter pattern. — indeterminately sequenced, Jul 03 '13 at 19:20
@indeterminatelysequenced This is the thing i want to avoid :) I don't want to have `a.h` included in users of `NewA`, it should hide all details of `A` inside it. And I don't want to create `A` objects on heap. — user2547823, Jul 03 '13 at 19:25

score 1 · Answer 1 · answered Jul 03 '13 at 20:32

In a comment you say that

I don't want to allocate A and hold A* pImpl, because it's already a wrapper around some pointer (AImpl)

Despite this requirement, you allocate a temporary A object in NewA::newMethodOfA(). In what way is this supposed to be better than allocating A once and just re-use it? Your solution is not good because 1) you create a new temporary A over and over again, and 2) you force the users of NewA to provide an instance of A instead of just creating one yourself.

I suggest you bite the bullet and just make a proper "PIMPL on top of PIMPL" implementation (as Captain Obvlious puts it):

// new_a.h
class A;

class NewA
{
    A* pImpl;

public:
    NewA();
    ~NewA();

    void newMethodOfA();
};

//new_a.cpp
#include "a.h"
NewA::NewA() : pImpl( new A() ) {}
NewA::~NewA() { delete pImpl; }

void NewA::newMethodOfA()
{
    pImpl->someMethodOfA();
}

This meets all your other requirements:

You don't want to have a.h included in new_a.h
You want to provide a modified interface of A so users of NewA won't know anything about A and AImpl
You want to hide the implementation of A

The only thing that doesn't quite meet up is that in the code you show the default constructor of A initializes its pImpl member to 0 - this is weird! Since when is the user of a PIMPL class required to provide the object that is wrapped by the PIMPL class? Cf. Wikipedia's Opaque Pointer article.

`A` object actually is a node of some tree, so there will be a lot of such objects created around. I don't want to allocate each of them on heap because it can hit performance. All real stuff is in `AImpl`. I suppose that creating temporary object on stack for this purpose is faster than dynamically allocate it, but i can be wrong. — user2547823, Jul 04 '13 at 07:16
I found [Fast Pimpl Idiom](http://www.gotw.ca/gotw/028.htm) that seems to be the best solution, but in this particular case as far as `A` exposes pointer to internal implementation, i think it will be easier to just use it... — user2547823, Jul 04 '13 at 08:22

Hide implementation details via internal pointer

1 Answers1