Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Pimpl idiom with inheritance

I want to use pimpl idiom with inheritance.

Here is the base public class and its implementation class:

class A
{
    public:
      A(){pAImpl = new AImpl;};
      void foo(){pAImpl->foo();};
    private:
      AImpl* pAImpl;  
};
class AImpl
{
    public:
      void foo(){/*do something*/};
};

And I want to be able to create the derived public class with its implementation class:

class B : public A
{
    public:
      void bar(){pAImpl->bar();};    // Can't do! pAimpl is A's private.
};        

class BImpl : public AImpl
{
    public:
      void bar(){/*do something else*/};
};

But I can't use pAimpl in B because it is A's private.

So I see some ways to solve it:

  1. Create BImpl* pBImpl member in B, and pass it to A with additional A constructor, A(AImpl*).
  2. Change pAImpl to be protected (or add a Get function), and use it in B.
  3. B shouldn't inherit from A. Create BImpl* pBImpl member in B, and create foo() and bar() in B, that will use pBImpl.
  4. Any other way?

What should I choose?

like image 274
Igor Avatar asked Jan 29 '09 11:01

Igor


2 Answers

class A
{
    public:
      A(bool DoNew = true){
        if(DoNew)
          pAImpl = new AImpl;
      };
      void foo(){pAImpl->foo();};
    protected:
      void SetpAImpl(AImpl* pImpl) {pAImpl = pImpl;};
    private:
      AImpl* pAImpl;  
};
class AImpl
{
    public:
      void foo(){/*do something*/};
};

class B : public A
{
    public:
      B() : A(false){
          pBImpl = new BImpl;
          SetpAImpl(pBImpl);
      };
      void bar(){pBImpl->bar();};    
    private:
      BImpl* pBImpl;  
};        

class BImpl : public AImpl
{
    public:
      void bar(){/*do something else*/};
};
like image 105
Igor Avatar answered Sep 25 '22 08:09

Igor


I think the best way from a purely object-oriented-theoretical perspective is to not make BImpl inherit from AImpl (is that what you meant in option 3?). However, having BImpl derive from AImpl (and passing the desired impl to a constructor of A) is OK as well, provided that the pimpl member variable is const. It doesn't really matter whether you use a get functions or directly access the variable from derived classes, unless you want to enforce const-correctness on the derived classes. Letting derived classes change pimpl isn't a good idea - they could wreck all of A's initialisation - and nor is letting the base class change it a good idea. Consider this extension to your example:

class A
{
protected:
   struct AImpl {void foo(); /*...*/};
   A(AImpl * impl): pimpl(impl) {}
   AImpl * GetImpl() { return pimpl; }
   const AImpl * GetImpl() const { return pimpl; }
private:
   AImpl * pimpl;
public:
   void foo() {pImpl->foo();}


   friend void swap(A&, A&);
};

void swap(A & a1, A & a2)
{
   using std::swap;
   swap(a1.pimpl, a2.pimpl);
}

class B: public A
{
protected:
   struct BImpl: public AImpl {void bar();};
public:
   void bar(){static_cast<BImpl *>(GetImpl())->bar();}
   B(): A(new BImpl()) {}

};

class C: public A
{
protected:
   struct CImpl: public AImpl {void baz();};
public:
   void baz(){static_cast<CImpl *>(GetImpl())->baz();}
   C(): A(new CImpl()) {}
};

int main()
{
   B b;
   C c;
   swap(b, c); //calls swap(A&, A&)
   //This is now a bad situation - B.pimpl is a CImpl *, and C.pimpl is a BImpl *!
   //Consider:
   b.bar(); 
   //If BImpl and CImpl weren't derived from AImpl, then this wouldn't happen.
   //You could have b's BImpl being out of sync with its AImpl, though.
}

Although you might not have a swap() function, you can easily conceive of similar problems occurring, particularly if A is assignable, whether by accident or intention. It's a somewhat subtle violation of the Liskov substitutability principle. The solutions are to either:

  1. Don't change the pimpl members after construction. Declare them to be AImpl * const pimpl. Then, the derived constructors can pass an appropriate type and the rest of the derived class can downcast confidently. However, then you can't e.g. do non-throwing swaps, assignments, or copy-on-write, because these techniques require that you can change the pimpl member. However however, you're probably not really intending to do these things if you have an inheritance hierarchy.

  2. Have unrelated (and dumb) AImpl and BImpl classes for A and B's private variables, respectively. If B wants to do something to A, then use A's public or protected interface. This also preserves the most common reason to using pimpl: being able to hide the definition of AImpl away in a cpp file that derived classes can't use, so half your program doesn't need to recompile when A's implementation changes.

like image 36
Doug Avatar answered Sep 23 '22 08:09

Doug