CODEWITHSUNDEEP
c++pimpl-idiom
user2547823
user2547823

Reputation: 11

Hide implementation details via internal pointer

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.

Upvotes: 0

Views: 255

Answers (1)

herzbube
herzbube

Reputation: 13378

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:

  1. You don't want to have a.h included in new_a.h
  2. You want to provide a modified interface of A so users of NewA won't know anything about A and AImpl
  3. 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.

Upvotes: 1

Related Questions