PIMPL problem: How to have multiple interfaces to the impl w/o code duplication

Question

I have this pimpl design where the implementation classes are polymorphic but the interfaces are supposed to just contain a pointer, making them polymorphic somewhat defeats the purpose of the design.

So I create my Impl and Intf base classes to provide reference counting. And then the user can create their implementations. An example:

class Impl {
    mutable int _ref;
public:
    Impl() : _ref(0) {}
    virtual ~Impl() {}

    int addRef() const { return ++_ref; }
    int decRef() const { return --_ref; }
};

template <typename TImpl>
class Intf {
    TImpl* impl;
public:
    Intf(TImpl* t = 0) : impl(0) {}
    Intf(const Intf& other) : impl(other.impl) { if (impl) impl->addRef(); }
    Intf& operator=(const Intf& other) {
         if (other.impl) other.impl->addRef();
         if (impl && impl->decRef() <= 0) delete impl;
         impl = other.impl;
    }
    ~Intf() { if (impl && impl->decRef() <= 0) delete impl; }
protected:
    TImpl* GetImpl() const { return impl; }
    void SetImpl(... //etc
};

class ShapeImpl : public Impl {
public:
    virtual void draw() = 0;
};

class Shape : public Intf<ShapeImpl> {
public:
    Shape(ShapeImpl* i) : Intf<ShapeImpl>(i) {}

    void draw() {
         ShapeImpl* i = GetImpl();
         if (i) i->draw();
    }
};

class TriangleImpl : public ShapeImpl {
public:
    void draw();
};

class PolygonImpl : public ShapeImpl {
public:
    void draw();
    void addSegment(Point a, Point b);
};

Here is where have the issue. There are two possible declaration for class Polygon:

class Polygon1 : public Intf<PolygonImpl> {
public:
    void draw() {
         PolygonImpl* i = GetImpl();
         if (i) i->draw();
    }
    void addSegment(Point a, Point b) {
        PolygonImpl* i = GetImpl();
        if (i) i->addSegment(a,b);
    }
};

class Polygon2 : public Shape {
    void addSegment(Point a, Point b) {
        ShapeImpl* i = GetImpl();
        if (i) dynamic_cast<Polygon*>(i)->addSegment(a,b);
    }
}

In the Polygon1, I have rewrite the code for draw because I have not inherited it. In Polygon2 I need ugly dynamic casts because GetImpl() doesn't know about PolygonImpl. What I would like to do is something like this:

template <typename TImpl>
struct Shape_Interface {
    void draw() {
        TImpl* i = GetImpl();
        if (i) i->draw();
    }
};

template <typename TImpl>
struct Polygon_Interface : public Shape_Interface<Timpl> {
    void addSegment(Point a, Point b) { ... }
};

class Shape : public TIntf<ShapeImpl>, public Shape_Interface<ShapeImpl> {...};

class Polygon : public TIntf<PolygonImpl>, public Polygon_Interface<PolygonImpl> {
public:
    Polygon(PolygonImpl* i) : TIntf<PolygonImpl>(i) {}
};

But of course there's a problem here. I can't access GetImpl() from the Interface classes unless I derive them from Intf. And if I do that, I need to make Intf virtual everywhere it appears.

template <typename TImpl>
class PolygonInterface : public virtual Intf<TImpl> { ... };

class Polygon : public virtual Intf<PolygonImpl>, public PolygonInterface { ... }

OR I can store a TImpl*& in each Interface and construct them with a reference to the base Intf::impl. But that just means I have a pointer pointing back into myself for every interface included.

template <typename TImpl>
class PolygonInterface {
    TImpl*& impl;
public:
    PolygonInterface(TImpl*& i) : impl(i) {}
...};

Both of these solutions bloat the Intf class, add an extra dereference, and basically provide no benefit over straight polymorphism.

So, the question is, is there a third way, that I've missed that would solve this issue besides just duplicating the code everywhere (with its maintenance issues)?

TOTALLY SHOULD, BUT DOESN'T WORK: I wish there were base classes unions that just overlaid the class layouts and, for polymorphic classes, required that they have the exact same vtable layout. Then both Intf and ShapeInterface would each declare a single T* element and access it identically:

class Shape : public union Intf<ShapeImpl>, public union ShapeInterface<ShapeImpl> {};

Answer 1

I should note that your Impl class is nothing more than the reimplementation of a shared_ptr without the thread safety and all those cast bonuses.

Pimpl is nothing but a technic to avoid needless compile-time dependencies.

You do not need to actually know how a class is implemented to inherit from it. It would defeat the purpose of encapsulation (though your compiler does...).

So... I think that you are not trying to use Pimpl here. I would rather think this is a kind of Proxy patterns, since apparently:

Polygon1 numberOne;
Polygon2 numberTwo = numberOne;

numberTwo.changeData(); // affects data from numberOne too
                        // since they point to the same pointer!!

If you want to hide implementation details

Use Pimpl, but the real one, it means copying in depth during copy construction and assignment rather than just passing the pointer around (whether ref-counted or not, though ref-counted is preferable of course :) ).

If you want a proxy class

Just use a plain shared_ptr.

For inheritance

It does not matter, when you inherit from a class, how its private members are implemented. So just inherit from it.

If you want to add some new private members (usual case), then:

struct DerivedImpl;

class Derived: public Base // Base implemented with a Pimpl
{
public:

private:
  std::shared_ptr<DerivedImpl> _data;
};

There is not much difference with classic implementation, as you can see, just that there is a pointer in lieu of a bunch of data.

BEWARE

If you forward declare DerivedImpl (which is the goal of Pimpl), then the destructor automatically generated by the compiler is... wrong.

The problem is that in order to generate the code for the destructor, the compiler needs the definition of DerivedImpl (ie: a complete type) in order to know how to destroy it, since a call to delete is hidden in the bowels of shared_ptr. However it may only generate a warning at compilation time (but you'll have a memory leak).

Furthermore, if you want an in-depth copy (rather than a shallow one, which consists in the copy and the original both pointing to the same DerivedImpl instance), you will also have to define manually the copy-constructor AND the assignment operator.

You may decide to create a better class that shared_ptr which will have deep-copy semantics (which could be called member_ptr as in cryptopp, or just Pimpl ;) ). This introduce a subtle bug though: while the code generated for the copy-constructor and the assignement operator could be thought of as correct, they are not, since once again you need a complete type (and thus the definition of DerivedImpl), so you will have to write them manually.

This is painful... and I'm sorry for you.

EDIT: Let's have a Shape discussion.

// Shape.h
namespace detail { class ShapeImpl; }

class Shape
{
public:
  virtual void draw(Board& ioBoard) const = 0;
private:
  detail::ShapeImpl* m_impl;
}; // class Shape


// Rectangle.h
namespace detail { class RectangleImpl; }

class Rectangle: public Shape
{
public:
  virtual void draw(Board& ioBoard) const;

  size_t getWidth() const;
  size_t getHeight() const;
private:
  detail::RectangleImpl* m_impl;
}; // class Rectangle


// Circle.h
namespace detail { class CircleImpl; }

class Circle: public Shape
{
public:
  virtual void draw(Board& ioBoard) const;

  size_t getDiameter() const;
private:
  detail::CircleImpl* m_impl;
}; // class Circle

You see: neither Circle nor Rectangle care if Shape uses Pimpl or not, as its name implies, Pimpl is an implementation detail, something private that is not shared with the descendants of the class.

And as I explained, both Circle and Rectangle use Pimpl too, each with their own 'implementation class' (which can be nothing more than a simple struct with no method by the way).

Answer 2

I have seen lots of solutions to this basic conundrum: polymorphism + variation in interfaces.

One basic approach is to provide a way to query for extended interfaces - so you have something along the lines of COM programming under Windows:

const unsigned IType_IShape = 1;
class IShape
{
public:
    virtual ~IShape() {} // ensure all subclasses are destroyed polymorphically!

    virtual bool isa(unsigned type) const { return type == IType_IShape; }

    virtual void Draw() = 0;
    virtual void Erase() = 0;
    virtual void GetBounds(std::pair<Point> & bounds) const = 0;
};


const unsigned IType_ISegmentedShape = 2;
class ISegmentedShape : public IShape
{
public:
    virtual bool isa(unsigned type) const { return type == IType_ISegmentedShape || IShape::isa(type); }

    virtual void AddSegment(const Point & a, const Point & b) = 0;
    virtual unsigned GetSegmentCount() const = 0;
};

class Line : public IShape
{
public:
    Line(std::pair<Point> extent) : extent(extent) { }

    virtual void Draw();
    virtual void Erase();
    virtual void GetBounds(std::pair<Point> & bounds);

private:
    std::pair<Point> extent;
};


class Polygon : public ISegmentedShape
{
public:
    virtual void Draw();
    virtual void Erase();
    virtual void GetBounds(std::pair<Point> & bounds);
    virtual void AddSegment(const Point & a, const Point & b);
    virtual unsigned GetSegmentCount() const { return vertices.size(); }

private:
    std::vector<Point> vertices;
};

Another option would be to make a single richer base interface class - which has all the interfaces you need, and then to simply define a default, no-op implementation for those in the base class, which returns false or throws to indicate that it isn't supported by the subclass in question (else the subclass would have provided a functional implementation for this member function).

class Shape
{
public:

    struct Unsupported
    {
     Unsupported(const std::string & operation) : bad_op(operation) {}

     const std::string & AsString() const { return bad_op; }

     std::string bad_op;
    };


    virtual ~Shape() {} // ensure all subclasses are destroyed polymorphically!

    virtual void Draw() = 0;
    virtual void Erase() = 0;
    virtual void GetBounds(std::pair<Point> & bounds) const = 0;
    virtual void AddSegment(const Point & a, const Point & b) { throw Unsupported("AddSegment"); }
    virtual unsigned GetSegmentCount() const { throw Unsupported("GetSegmentCount"); }
};

I hope that this helps you to see some possibilities.

Smalltalk had the wonderful attribute of being able to ask the meta-type-system whether a given instance supported a particular method - and it supported having a class-handler that could execute anytime a given instance was told to perform an operation it didn't support - along with what operation that was, so you could forward it as a proxy, or you could throw a different error, or simply quietly ignore that operation as a no-op).

Objective-C supports all of those same modalities as Smalltalk! Very, very cool things can be accomplished by having access to the type-system at runtime. I assume that .NET can pull of some crazy cool stuff along those lines (though I doubt that its nearly as elegant as Smalltalk or Objective-C, from what I've seen).

Anyway, ... good luck :)

Answer 3

I think you were right in that I didn't understand your question initially.

I think you're trying to force a square shape into a round hole... it don't quite fit C++.

You can force that your container holds pointers to objects of a given base-layout, and then allow objects of arbitrary composition to be actually pointed to from there, assuming that you as a programmer only actually place objects that in fact have identical memory layouts (member-data - there's no such thing as member-function-layout for a class unless it has virtuals, which you wish to avoid).

std::vector< boost::shared_ptr<IShape> > shapes;

NOTE at the absolute MINIMUM, you must still have a virtual destructor defined in IShape, or object deletion is going to fail miserably

And you could have classes which all take a pointer to a common implementation core, so that all compositions can be initialized with the element that they share (or it could be done statically as a template via pointer - the shared data).

But the thing is, if I try to create an example, I fall flat the second I try to consider: what is the data shared by all shapes? I suppose you could have a vector of Points, which then could be as large or small as any shape required. But even so, Draw() is truly polymorphic, it isn't an implementation that can possibly be shared by multiple types - it has to be customized for various classifications of shapes. i.e. a circle and a polygon cannot possibly share the same Draw(). And without a vtable (or some other dynamic function pointer construct), you cannot vary the function called from some common implementation or client.

Your first set of code is full of confusing constructs. Maybe you can add a new, simplified example that PURELY shows - in a more realistic way - what you're trying to do (and ignore the fact that C++ doesn't have the mechanics you want - just demonstrate what your mechanic should look like).

To my mind, I just don't get the actual practical application, unless you're tyring to do something like the following:

Take a COM class, which inherits from two other COM Interfaces:

class MyShellBrowserDialog : public IShellBrowser, public ICommDlgBrowser
{
  ...
};

And now I have a diamond inheritence pattern: IShellBrowser inherits ultimately from IUnknown, as does ICommDlgBrowser. But it seems incredibly silly to have to write my own IUnknown:AddRef and IUnknown::Release implementation, which is a highly standard implementation, because there's no way to cause the compiler to let another inherited class supply the missing virtual functions for IShellBrowser and/or ICommDlgBrowser.

i.e., I end up having to:

class MyShellBrowserDialog : public IShellBrowser, public ICommDlgBrowser
{
public:
 virtual ULONG STDMETHODCALLTYPE AddRef(void) { return ++m_refcount; }
 virtual ULONG STDMETHODCALLTYPE Release(void) { return --m_refcount; }
...
}

because there's no way I know of to "inherit" or "inject" those function implementations into MyShellBrowserDialog from anywhere else which actually fill-in the needed virtual member function for either IShellBrowser or ICommDlgBrowser.

I can, if the implementations were more complex, manually link up the vtable to an inherited implementor if I wished:

class IUnknownMixin
{
 ULONG m_refcount;
protected:
 IUnknonwMixin() : m_refcount(0) {}

 ULONG AddRef(void) { return ++m_refcount; } // NOTE: not virutal
 ULONG Release(void) { return --m_refcount; } // NOTE: not virutal
};

class MyShellBrowserDialog : public IShellBrowser, public ICommDlgBrowser, private IUnknownMixin
{
public:
 virtual ULONG STDMETHODCALLTYPE AddRef(void) { return IUnknownMixin::AddRef(); }
 virtual ULONG STDMETHODCALLTYPE Release(void) { return IUnknownMixin::Release(); }
...
}

And if I needed the mix-in to actually refer to the most-derived class to interact with it, I could add a template parameter to IUnknownMixin, to give it access to myself.

But what common elements could my class have or benefit by that IUnknownMixin couldn't itself supply?

What common elements could any composite class have that various mixins would want to have access to, which they needed to derive from themselves? Just have the mixins take a type parameter and access that. If its instance data in the most derived, then you have something like:

template <class T>
class IUnknownMixin
{
 T & const m_outter;
protected:
 IUnknonwMixin(T & outter) : m_outter(outter) {}
 // note: T must have a member m_refcount

 ULONG AddRef(void) { return ++m_outter.m_refcount; } // NOTE: not virtual
 ULONG Release(void) { return --m_outter.m_refcount; } // NOTE: not virtual
};

Ultimately your question remains somewhat confusing to me. Perhaps you could create that example that shows your preferred-natural-syntax that accomplishes something clearly, as I just don't see that in your initial post, and I can't seem to sleuth it out from toying with these ideas myself.