7

I've been playing with the Pimpl idiom and reaping all sorts of benefits from it. The only thing I haven't been too keen on is the feeling I get when I define the functions.

  • Once in the header (P def)
  • Once at the top of the .cpp (Impl def)
  • Once in the middle of the .cpp (Impl Impl)
  • Once at the lower end of the .cpp (P Impl)

I really enjoy cutting down code disparity and redundancy, and I feel like my code is less than well oiled when I have to add or change functions in even relatively complex Impls in my current project.

My question is, what effective ways are there to imply or template my classes in such a way that if I were to define a new function, I'd only have to write one explicit definition and implementation, and have the rest remain spatially close to the explicits in code; and if I were to change a function, the changes necessary would be as few as possible?

  • 3
    It may be very clear for you but I am afraid it is unclear to me. Do you have code examples to show off the troubles you face ? – Matthieu M. Jul 08 '14 at 15:08
  • Why do you define those functions in Impl at all? Declare a function in the header, and then define it in "lower end of the .cpp". – Wojtek Surowka Jul 08 '14 at 15:08
  • 1
    @WojtekSurowka _'Declare a function in the header,'_ That wouldn't be very useful using the [Pimpl idiom](http://en.wikipedia.org/wiki/Opaque_pointer), it's all about it's point, that the implementation isn't public in the header. – πάντα ῥεῖ Jul 08 '14 at 15:29
  • 1
    @πάνταῥεῖ Wojtek is correct in that `public` functions can be defined once in the .cpp if they refer to what would have been `private` members ala `p_impl_->xxx`. It's not *necessarily* the case that the implementation class needs to have member functions at all, though the more complicated the class the more likely it is that that's more convenient - it's also possible to e.g. pass `p_impl_` to a non-member function in an anonymous namespace if a "private" function is wanted. – Tony Delroy Jul 08 '14 at 15:37
  • 1
    @πάνταῥεῖ I did not say "define" I said "declare". Functions need to be declared in the header, the definition when Pimpl is used will be in cpp. – Wojtek Surowka Jul 08 '14 at 15:52
  • @WojtekSurowka Sounds like you didn't get the pattern. Even _declaration_ of the implementation class **isn't wanted in the header** in case of using Pimpl idiom. – πάντα ῥεῖ Jul 08 '14 at 16:05
  • @πάνταῥεῖ Sounds like you didn't understand my comment. I said nothing about declaration of Pimpl class. What I said was: if you need a function, declare it in the header, define it in cpp. No need for the function in Pimpl at all. – Wojtek Surowka Jul 08 '14 at 16:09
  • 1
    @WojtekSurowka Ah, sorry! I meant you were talking about the pimpl class/functions themselves. What you said seemed so elf-evident for me, I didn't even consider someone mentions this. That's the crux with unclear questions :(. Sorry again. – πάντα ῥεῖ Jul 08 '14 at 16:13
  • @πάνταῥεῖ No problem :) – Wojtek Surowka Jul 08 '14 at 16:18
  • 3
    Can you explain the difference between an agile pimple and a not agile pimple? – nwp Jul 08 '14 at 16:24
  • 1
    @nwp An agile pimple can appear in a different location on your face when you wake up, but a non-agile pimple cannot. – MarkB Jul 08 '14 at 21:48
  • I don't understand why this was closed, its seems very clear and got some proper answers. – Galik Jul 05 '15 at 09:49

4 Answers4

11

You might consider something along these lines:

An Interface class to minimize repeating declarations. The client will use the PublicImplementation class in their code.

Pimpl.h

#ifndef PIMPL_H_
#define PIMPL_H_

#include <memory> // std::unique_ptr

class Interface
{
public:
    virtual ~Interface() {}

    virtual void func_a() = 0;
    virtual void func_b() = 0;
};

class PublicImplementation
{
    // smart pointer provides exception safety
    std::unique_ptr<Interface> impl;

public:
    PublicImplementation();

    // pass-through invoker
    Interface* operator->() { return impl.get(); }
};

#endif // PIMPL_H_

Pimpl.cpp

#include "Pimpl.h"
#include <iostream>

class PrivateImplementation
: public Interface
{
public:

    void func_a() override { std::cout << "a" << '\n'; }
    void func_b() override { std::cout << "b" << '\n'; }
};

PublicImplementation::PublicImplementation()
: impl(new PrivateImplementation)
{
}

And finally this is what the client code does:

Main.cpp

#include "Pimpl.h"

int main()
{
    PublicImplementation pi; // not a pointer

    pi->func_a(); // pointer semantics
    pi->func_b();
}
Galik
  • 42,526
  • 3
  • 76
  • 100
  • 2
    You can use `std::unique_ptr impl;` to keep PublicInterface tidier and provide exception safety. Also that would fix your existing Rule of Three problem. – M.M Mar 07 '15 at 09:42
  • @MattMcNabb I have updated the example to include your suggestion. – Galik Jun 16 '15 at 16:48
  • 1
    Clean and useful, just what I was looking for... thank you a lot – Oneiros Feb 28 '17 at 08:07
3

Let's postulate your header starts something like this:

class X
{
  public:
    ...la de dah...
  private:
    struct Impl;
    Impl* p_impl_;
};

Then when you add functions you have a choice to make:

  1. do you have the X member function definition implement the logic, referring to p_impl_-> things all over the place, or

  2. return p_impl->same_fn(all_the_args); and keep the logic inside the Impl class?

If you choose 1. then you end up with a function declaration in the header, and a (slightly messier than usual) definition in the matching implementation file.

If you choose 2. then you end up with a function declaration in the header file, a wrapping/forwarding definition in the matching implementation file, and at a minimum a definition in the Impl structure (I tend not to define the functions outside the Impl class definition - it's an implementation detail and the interface is not public anyway).

There is no generally desirable way to improve on this situation (i.e. macro hackery and extra code-generation scripts in your build process may occasionally be warranted, but very rarely).


It may not matter a whole heap, though it may be of interest that a variation on the second approach is to first implement a class that doesn't use the pimpl idiom (complete with proper header and optionally inline functions), you can then wrap it with a pimpl management object and forward functions to it, and in that way you keep the freedom to have some code somewhere some day decide it wants to use the functionality without using the pimpl wrapper, perhaps for improved performance / reduced memory usage at the cost of the recompilation dependency. You can also do this to make use of a specific instantiation of a template without exposing the template's code.

To illustrate this option (as requested in a comment), let's start with a silly non-pimpl class X in its own files, then create a Pimpl::X wrapper (the use of namespace and the same class name is entirely optional but facilitates flipping client code to use either, and a reminder - this isn't meant to be concise, the point here is to let a non-pImpl version be usable too):

// x.h
class X
{
  public:
    int get() const { return n_; }   // inline
    void operator=(int);  // out-of-line definition
  private:
    int n_;
};

// x.c++
#include <x.h>
void X::operator=(int n) { n_ = n * 2; }

// x_pimpl.h
namespace Pimpl
{
    class X
    {
      public:
        X();
        X(const X&);
        ~X();
        X& operator=(const X&);
        int get() const;
        void operator=(int);
      private:
        struct Impl;
        Impl* p_impl_;
    };
}

x_pimpl.c++
#include <x.h>
namespace Pimpl
{
    struct X::Impl
    {
        ::X x_; 
    };

    // the usual handling...
    X() : p_impl_(new Impl) { }
    X(const X& rhs) : p_impl(new Impl) { p_impl_->x_ = rhs.p_impl_->x_; }
    ~X() { delete p_impl_; }
    X& operator=(const X& rhs) { p_impl_->x_ = rhs.p_impl_->x_; return *this; }

    // the wrapping...
    int X::get() const { return p_impl_->x_.get(); }
    void X::operator=(int n) { p_impl_->x_ = n; }
}

If you opt for the above variation on 2, which makes the "implementation" a usable entity in it's own right, then yes - you may end up with 2 declarations and 2 definitions related to a single function, but then one of the definitions will be a simple wrapper/forwarding function which is only significantly repetitive and tedious if the functions are very short and numerous but have lots of parameters.

Tony Delroy
  • 94,554
  • 11
  • 158
  • 229
  • I really do like the idea of a potentially independent implementation. If you don't mind, could you expand your code for that option? I'd be happy to accept that as an agile pimpl. –  Jul 08 '14 at 15:47
3

There's no requirement to treat the IMPL object to the same rules & standards as an object declaration in the .h file. By allowing member variables to be public (via a struct declaration), you don't need to implement an unnecessary wrapper layer. This is generally safe, since only the .cpp file has access to IMPL anyway.

Consider the following code that achieves the benefits of the PIMPL idiom without unnecessary code duplication:

// x.h
class X {
public:
    X();
    ~X();

    X(const X&) = delete;
    X& operator =(const X&) = delete;

    void set(int val);
    int get() const;

private:
    struct IMPL;
    IMPL* impl;
};

// x.cpp
#include "x.h"

struct X::IMPL {
    int val;
};


X::X() : impl(new IMPL) {}

X::~X() { delete impl; }

void X::set(int val)
{
    impl->val = val;
}

int X::get() const
{
    return impl->val;
}

// main.cpp
#include <iostream>
#include "x.h"

int main (int, char *[])
{
    X x;
    x.set(10);
    std::cout << x.get() << std::endl;
    return 0;
}
MarkB
  • 852
  • 4
  • 13
2

I'm just going to start by sumarizing to make sure I understand: You like the benefits of using pimpl, but dislike the amount of boilerplate code when adding or modifying functions?

In a nutshell, there is no template magic you can use to eliminate this boilerplate, but there are things to consider here as well:

  • You write code only once but read it many times, and you have at your disposal a variety of copy-paste capabilities. Initially creating the function isn't the majority of the time you will spend on this class. Compiling and maintaining is where your time will be spent.
  • Be sure to keep the public class API as simple as possible. The fewer functions you have in the public API the less boilerplate you have to write. You can make as many functions as you like in the impl and y ou only have to modify them there.
  • If you find yourself changing the public class API many many times, you might wish to slightly adjust your design process. Spend ten more minutes up front looking at/writing down use cases and you may reduce your API changes by 90%.
Mark B
  • 91,641
  • 10
  • 102
  • 179