Pimpl Idiom

Discussion in 'C++' started by Icosahedron, Nov 20, 2003.

  1. Icosahedron

    Icosahedron Guest

    I've been going through some old code trying to clean it up
    and rearchitect it based on more modern C++ idioms. In the
    old code I often used the Pimpl idiom on a class by class
    basis, creating impl within a class as such:

    a.h
    class A {
    ...stuff...
    struct AImpl;
    AImpl* _impl;
    };

    a.cpp

    struct AImpl {
    ...stuff...
    };

    After doing this in a number of classes, I thought it would
    be "cool" to have a single Pimpl class that any class
    wishing to define implementation specifics could inherit
    from and then define a class specific Impl class as such:

    pimpl.h

    class Pimpl {
    struct Impl;
    boost::shared_ptr<Impl> _impl;
    };

    a.h

    class A : public Pimpl {
    ... stuff ...
    };

    a.cpp

    struct A::Impl {
    ... stuff ...
    };

    This compiled okay, but then I ran into an inheritance
    problem (as many of you probably predicted just reading
    this):

    b.h

    class B : public A, public Pimpl {
    };

    I could use virtual inheritance, but I want 2 impl classes,
    one for A's implementation specific stuff and B's
    implementation specific stuff. Of course, I thought I would
    use templates to parameterize each one to give it a
    different type so.

    pimpl.h

    template <typename T>
    class Pimpl {
    struct Impl;
    boost::shared_ptr<Impl> _impl;
    };

    a.h

    class A : public Pimpl<A> {
    };

    b.h

    class B : public A, public Pimpl<B> {
    };


    Still, this has not quite gotten me where I want to be. I
    would like to be able to have an A::_impl and a B::_impl
    that I can distinguish inside of B based on the methods that
    have to be called. The other is that, if possible, I wish
    to refer to them as A::_impl rather than Pimpl<A>::_impl and
    likewise for B.

    Any suggestions? Is this whole thing just bad form? Should
    I just stick with the original approach?

    Jay
     
    Icosahedron, Nov 20, 2003
    #1
    1. Advertising

  2. Icosahedron

    Icosahedron Guest

    "Icosahedron" <> wrote in message
    news:q00vb.294935$...
    > I've been going through some old code trying to clean it up
    > and rearchitect it based on more modern C++ idioms. In the
    > old code I often used the Pimpl idiom on a class by class
    > basis, creating impl within a class as such:
    >
    > a.h
    > class A {
    > ...stuff...
    > struct AImpl;
    > AImpl* _impl;
    > };
    >
    > a.cpp
    >
    > struct AImpl {
    > ...stuff...
    > };


    Here I meant A::AImpl of course.

    The idea behind "unifying" all the different Pimpls is that
    a number of the impl classes must talk to each other within
    the module. Rather than have friend declarations scattered
    all throughout the heirarchy, I thought it would be better
    to have a single Pimpl and make it a friend of itself, so
    that the classes could interact within the module freely.

    > pimpl.h
    >
    > class Pimpl {
    > struct Impl;
    > boost::shared_ptr<Impl> _impl;
    > };
    >
    > a.h
    >
    > class A : public Pimpl {
    > ... stuff ...
    > };
    >
    > a.cpp
    >
    > struct A::Impl {
    > ... stuff ...
    > };
    >
    > This compiled okay, but then I ran into an inheritance
    > problem (as many of you probably predicted just reading
    > this):
    >
    > b.h
    >
    > class B : public A, public Pimpl {
    > };
    >
    > I could use virtual inheritance, but I want 2 impl classes,
    > one for A's implementation specific stuff and B's
    > implementation specific stuff. Of course, I thought I would
    > use templates to parameterize each one to give it a
    > different type so.
    >
    > pimpl.h
    >
    > template <typename T>
    > class Pimpl {
    > struct Impl;
    > boost::shared_ptr<Impl> _impl;
    > };
    >
    > a.h
    >
    > class A : public Pimpl<A> {
    > };
    >
    > b.h
    >
    > class B : public A, public Pimpl<B> {
    > };
    >
    >
    > Still, this has not quite gotten me where I want to be. I
    > would like to be able to have an A::_impl and a B::_impl
    > that I can distinguish inside of B based on the methods that
    > have to be called. The other is that, if possible, I wish
    > to refer to them as A::_impl rather than Pimpl<A>::_impl and
    > likewise for B.


    Of course, having two _impl members is impossible, but
    having two GetImpl functions, each one with its own return
    value would be possible, each addressed by its class prefix
    (Pimpl<A>::GetImpl() and Pimpl<B>::GetImpl()) is what I'm
    thinking for those classes which have two impls.

    > Any suggestions? Is this whole thing just bad form? Should
    > I just stick with the original approach?


    Again, the above question still remains in effect.

    Jay
     
    Icosahedron, Nov 20, 2003
    #2
    1. Advertising

  3. Icosahedron

    David Rubin Guest

    Icosahedron wrote:
    > I've been going through some old code trying to clean it up
    > and rearchitect it based on more modern C++ idioms. In the
    > old code I often used the Pimpl idiom on a class by class
    > basis, creating impl within a class as such:


    [snip]
    > class A {
    > ...stuff...
    > struct AImpl;
    > AImpl* _impl;
    > };


    [snip]
    > After doing this in a number of classes, I thought it would
    > be "cool" to have a single Pimpl class that any class
    > wishing to define implementation specifics could inherit
    > from and then define a class specific Impl class as such:


    [snip]
    > class Pimpl {
    > struct Impl;
    > boost::shared_ptr<Impl> _impl;
    > };


    [snip]
    > class A : public Pimpl {
    > ... stuff ...
    > };


    [snip - problem with multiple inheritance and general confusion]
    > Any suggestions? Is this whole thing just bad form? Should
    > I just stick with the original approach?


    The thing is, you don't want to inherit an implementation interface, you
    want a *pointer* to an implementation object. This allows you to keep
    the interface and implementation separate, alleviating compile time
    dependencies, and improving encapsulation. So, your entire question is
    ill concieved, IMO. Stick to the original plan.

    /david

    --
    "As a scientist, Throckmorton knew that if he were ever to break wind in
    the echo chamber, he would never hear the end of it."
     
    David Rubin, Nov 20, 2003
    #3
  4. Icosahedron

    Icosahedron Guest

    > [snip - problem with multiple inheritance and general confusion]
    > > Any suggestions? Is this whole thing just bad form? Should
    > > I just stick with the original approach?

    >
    > The thing is, you don't want to inherit an implementation interface, you
    > want a *pointer* to an implementation object. This allows you to keep
    > the interface and implementation separate, alleviating compile time
    > dependencies, and improving encapsulation. So, your entire question is
    > ill concieved, IMO. Stick to the original plan.


    A good point. The reason I chose to do this (and not explained in the
    original post) was that in a suite of classes I am using, often one class'
    impl must gain
    access to another classes impl, but might only have the reference to the
    envelope class.

    class A {
    private:
    struct Impl;
    Impl* _impl;
    friend class C;
    };

    class C {
    public:
    void DoSomething(A* a);
    private:
    struct Impl;
    Impl* _impl;
    };

    where C might need to get something out of A's impl or call a method on it.

    C::DoSomething(A* a)
    {
    a->_impl->DoImplSpecificStuff();
    }

    How would you design this so that the friend statements aren't necessary?
    Since the impls are hidden anyways, making them public really doesn't expose
    any functionality, so I could do it that way, but of course that's not
    really
    elegant. I appreciate the feedback.

    Thanks,

    Jay
     
    Icosahedron, Nov 21, 2003
    #4
  5. Icosahedron

    Jack Walker Guest

    "Icosahedron" <> wrote in message news:<L0kvb.91952$>...
    > > [snip - problem with multiple inheritance and general confusion]
    > > > Any suggestions? Is this whole thing just bad form? Should
    > > > I just stick with the original approach?

    > >
    > > The thing is, you don't want to inherit an implementation interface, you
    > > want a *pointer* to an implementation object. This allows you to keep
    > > the interface and implementation separate, alleviating compile time
    > > dependencies, and improving encapsulation. So, your entire question is
    > > ill concieved, IMO. Stick to the original plan.

    >
    > A good point. The reason I chose to do this (and not explained in the
    > original post) was that in a suite of classes I am using, often one class'
    > impl must gain
    > access to another classes impl, but might only have the reference to the
    > envelope class.
    >
    > class A {
    > private:
    > struct Impl;
    > Impl* _impl;
    > friend class C;
    > };
    >
    > class C {
    > public:
    > void DoSomething(A* a);
    > private:
    > struct Impl;
    > Impl* _impl;
    > };
    >
    > where C might need to get something out of A's impl or call a method on it.
    >
    > C::DoSomething(A* a)
    > {
    > a->_impl->DoImplSpecificStuff();
    > }
    >
    > How would you design this so that the friend statements aren't necessary?
    > Since the impls are hidden anyways, making them public really doesn't expose
    > any functionality, so I could do it that way, but of course that's not
    > really
    > elegant. I appreciate the feedback.
    >
    > Thanks,
    >
    > Jay


    This may be telling you that the pimpl idiom is not well suited to your application.

    Jack
     
    Jack Walker, Nov 21, 2003
    #5
  6. Icosahedron

    Icosahedron Guest

    "Icosahedron" <> wrote in message
    news:q00vb.294935$...
    > I've been going through some old code trying to clean it up
    > and rearchitect it based on more modern C++ idioms. In the
    > old code I often used the Pimpl idiom on a class by class
    > basis, creating impl within a class as such:
    >


    [snip]

    > Still, this has not quite gotten me where I want to be. I
    > would like to be able to have an A::_impl and a B::_impl
    > that I can distinguish inside of B based on the methods that
    > have to be called. The other is that, if possible, I wish
    > to refer to them as A::_impl rather than Pimpl<A>::_impl and
    > likewise for B.
    >
    > Any suggestions? Is this whole thing just bad form? Should
    > I just stick with the original approach?


    FWIW, I finally got what I wanted, more or less. Here's a sample showing
    what I've done so far. Amazing how simple these things always seem to boil
    down to.

    #include <iostream>

    using namespace std;

    template <typename T>
    struct Impl;

    class C;

    class A {
    struct Impl<A>* _impl;
    template <class T> friend class Impl;
    public:
    A();
    };

    class B : public A {
    struct Impl<B>* _impl;
    template <class T> friend class Impl;
    public:
    B();
    void Print(C*);

    };

    class C {
    struct Impl<C>* _impl;
    template <class T> friend class Impl;
    public:
    C();
    };


    struct Impl<A> {
    A* that;
    Impl(A *a) { that = a; }
    void Print(void) { cout << "In Impl<A>::print" << endl; }
    };

    struct Impl<B> {
    B* that;
    Impl(B *b) { that = b; }
    void Print(C* c);
    };

    struct Impl<C> {
    C* that;
    Impl(C *c) { that = c; }
    void Print(void) { cout << "In Impl<C>::print" << endl; }
    };

    A::A() : _impl(new Impl<A>(this)) {}
    B::B() : _impl(new Impl<B>(this)) {}
    C::C() : _impl(new Impl<C>(this)) {}

    void Impl<B>::print(C* c)
    {
    cout << "In Impl<B>::print" << endl;
    that->A::_impl->Print(); // shows accessing a base class' impl
    c->_impl->Print(); // shows accessing another class' impl
    }

    void B::print(C* c)
    {
    cout << "In B::print" << endl;
    _impl->Print(c); // forward to impl class
    }

    int main(void)
    {
    C c;
    B b;
    b.Print(&c);
    }

    As you may see, I dropped the Pimpl being inherited. I don't know why I was
    thinking it, but I was doing that to allow for the automatic declaration of
    the Impl class, which would force the implementer to define a nested Impl
    class. Unfortunately, there is no way to extend friendship between these
    classes to the nested Impl classes or the main classes well.

    As for the Pimpl idiom being correct or not for this application, I chose it
    because it is a cross platform application wherein I wanted the
    implementation specific details to be kept out of the interface classes
    (though truthfully the interface classes contain data common to all
    platforms too, so they are not pure interface classes). I could have just
    defined the methods themselves for the classes on a platform by platform
    basis (each implementation for A could have been in separate directories for
    instance), and just kept the data in the impl structures, which would have
    worked I believe, and still given me the same opacity with regards to the
    specifics, but I wanted the freedom to add methods instead of just helper
    functions, though the distinction is pretty moot.

    The main reason I wanted this was that in my particular application, a
    renderer, the impl classes need access to details in other impl classes
    (such as texture id from the TextureImpl for setting the current texture in
    the CameraImpl class), and I didn't want to have to have friends defined for
    every particular combination. (The friends were required because to get to
    the _impl member of another class required that you have access to the
    private members of that class.) I could have done it one of several ways I
    believe:

    1) make the _impl member public - this means anyone can access it, but
    without the interface, it's pretty hard to do anything with it. This also
    could be done also with a GetImpl member function. Opaque, but not guarded
    from modification well.

    2) friends. Of course there's the obvious friend class declaration, but as
    mentioned, for every shader for instance, the Camera::Impl would have to be
    put in, and likewise each Shader::Impl would have to be listed in Camera.
    And that has to be in the main header file, not the implementation files.
    Or, I could do as I did and make all the Impl classes friends by default
    with one line. Not perfect, but done only once and still get the benefits
    of hidden implementation and access restriction from the outside.

    Along with #2, I am considering this is being designed to allow extension by
    other developers (with their classes being first class members of the
    library as well rather than some hacked in extension mechanism), I wanted
    some way for them to add to the library without having to add their classes
    as friends to the main classes.

    So, the main mechanism is that you have to have an Impl<T> member named
    _impl and a friend declaration "template <class T> friend class Impl;" that
    will automatically define other Impl<T> classes in the module as friends of
    the class. I keep the Impl class definition in the Render namespace, to
    avoid any potential conflicts and to preserve some semblance of module
    integrity.

    Well, that's the short of it. Please feel free to comment, as I do read
    them, if I don't respond to each one. Thanks again.

    Jay

    P.S. The above code compiles fine on GCC 3.2.3
     
    Icosahedron, Nov 22, 2003
    #6
  7. Icosahedron

    Jeff Guest

    Hi Jay,

    > ... in a suite of classes I am using, often one class's impl must gain
    > access to another class's impl, but might only have the reference to the
    > envelope class.
    > [e.g.]
    > C::DoSomething(A* a)
    > { a->_impl->DoImplSpecificStuff(); }
    >
    > How would you design this so that the friend statements aren't necessary?


    This is a design issue. Imagine what you're saying: you're
    encapsulating stuff by making it private in class A, and then you're
    specifically breaking (or at least dramatically reducing) the
    encapsulation by having class C use private member functions in class
    A. The classes are now more tightly coupled ... a change in class A
    may necessitate a change in class C (suppose A::DoImplSpecificStuff()
    disappears, etc) If class C wants to be able to call a member
    function in class A, it should be public.

    Optionally, you could define a separate friend class of A that has the
    helper function in it, and have both A and C call that (I believe this
    is better than having C be a friend of A, since C itself doesn't
    really need to be a friend to /all/ of A, it just needs a specific
    function ... sorry, don't have time to really explain myself on this,
    and I'm kind of shooting from the hip, it might be a crappy idea).

    Anyhow, keep thinking about the design. I don't believe that
    "friendship" is the right answer here, and you can probably come up
    with something better that is more encapsulated. Good luck!

    Jeff
     
    Jeff, Nov 22, 2003
    #7
  8. Icosahedron

    Icosahedron Guest

    > This is a design issue. Imagine what you're saying: you're
    > encapsulating stuff by making it private in class A, and then you're
    > specifically breaking (or at least dramatically reducing) the
    > encapsulation by having class C use private member functions in class
    > A. The classes are now more tightly coupled ... a change in class A
    > may necessitate a change in class C (suppose A::DoImplSpecificStuff()
    > disappears, etc) If class C wants to be able to call a member
    > function in class A, it should be public.


    True. Actually, the only thing I need is access to the _impl class member
    so I can call impl specific functions from another impl specific class in
    the module, but I don't know of another mechanism to grant that.

    > Optionally, you could define a separate friend class of A that has the
    > helper function in it, and have both A and C call that (I believe this
    > is better than having C be a friend of A, since C itself doesn't
    > really need to be a friend to /all/ of A, it just needs a specific
    > function ... sorry, don't have time to really explain myself on this,
    > and I'm kind of shooting from the hip, it might be a crappy idea).
    >
    > Anyhow, keep thinking about the design. I don't believe that
    > "friendship" is the right answer here, and you can probably come up
    > with something better that is more encapsulated. Good luck!


    Thanks. I see what you're saying. I posted a general response to this.
    You are right about the classes being tightly coupled, and you are right
    about this breaking encapsulation, but at least in my mind it does so within
    the module, and so really isn't breaking modular encapsulation.

    As for the A->B->C link, I see what you're saying. I think that makes more
    complication within a module where things are tightly coupled by nature, but
    it is a good idea for keeping encapsulation.

    I'll have to rethink what I've got and perhaps come up with something else.
    I originally wrote a long essay about why what I was doing was the right
    thing, and I still think it is the easiest within the module, but perhaps I
    can rethink what I'm doing internally to make it not necessary.

    Jay
     
    Icosahedron, Nov 22, 2003
    #8
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Debajit  Adhikary
    Replies:
    2
    Views:
    2,084
    Christopher Benson-Manica
    Jul 15, 2004
  2. Peteris Krumins
    Replies:
    2
    Views:
    514
    Peteris Krumins
    Aug 31, 2005
  3. jimmy

    friendship and pImpl idiom

    jimmy, Feb 8, 2006, in forum: C++
    Replies:
    3
    Views:
    458
    Alf P. Steinbach
    Feb 9, 2006
  4. Noah Roberts

    pimpl idiom and singletons

    Noah Roberts, May 24, 2007, in forum: C++
    Replies:
    4
    Views:
    455
    Noah Roberts
    May 25, 2007
  5. Daniel Lidström
    Replies:
    15
    Views:
    654
    Brendon Costa
    Oct 31, 2007
Loading...

Share This Page