views:

1096

answers:

7

There have been a few questions on SO about the pimpl idiom, but I'm more curious about how often it is leveraged in practice.

I understand there are some trade-offs between performance and encapsulation, plus some debugging annoyances due to the extra redirection.

With that, is this something that should be adopted on a per-class, or an all-or-nothing basis? Is this a best-practice or personal preference?

I realize that's somewhat subjective, so let me list my top priorities:

  • Code clarity
  • Code maintainability
  • Performance

I always assume that I will need to expose my code as a library at some point, so that's also a consideration.

EDIT: Any other options to accomplish the same thing would be welcome suggestions.

A: 

This idiom helps greatly with compile time on large projects.

External link

This is good too

GregC
+15  A: 

I'd say that whether you do it per-class or on an all-or-nothing basis depends on why you go for the pimpl idiom in the first place. My reasons, when building a library, have been one of the following:

  • Wanted to hide implementation in order to avoid disclosing information (yes, it was not a FOSS project :)
  • Wanted to hide implementation in order to make client code less dependent. If you build a shared library (DLL), you can change your pimpl class without even recompiling the application.
  • Wanted to reduce the time it takes to compile the classes using the library.
  • Wanted to fix a namespace clash (or similar).

None of these reasons prompts for the all-or-nothing approach. In the first one, you only pimplize what you want to hide, whereas in the second case it's probably enough to do so for classes which you expect to change. Also for the third and fourth reason there's only benefit from hiding non-trivial members that in turn require extra headers (e.g., of a third-party library, or even STL).

In any case, my point is that I wouldn't typically find something like this too useful:

class Point {
  public:      
    Point(double x, double y);
    Point(const Point& src);
    ~Point();
    Point& operator= (const Point& rhs);

    void setX(double x);
    void setY(double y);
    double getX() const;
    double getY() const;

  private:
    class PointImpl;
    PointImpl* pimpl;
}

In this kind of a case, the tradeoff starts to hit you because the pointer needs to be dereferenced, and the methods cannot be inlined. However, if you do it only for less trivial classes, then the slight overhead can typically be tolerated without any problems.

Pukku
Please, please, please at least declare (if not define) an assignment operator and copy constructor in any pimple container. This code is a ticking bomb.
Pontus Gagge
@Pontus Gagge: Added – thanks for the remark.
Pukku
A: 

I generally use it when I want to avoid a header file polluting my codebase. Windows.h is the perfect example. It is so badly behaved, I'd rather kill myself than have it visible everywhere. So assuming you want a class-based API, hiding it behind a pimpl class neatly solves the problem. (If you're content to just expose individual function, those can just be forward declared, of course, without putting them into a pimpl class)

I wouldn't use pimpl everywhere, partly because of the performance hit, and partly just because it's a lot of extra work for a usually small benefit. The main thing it gives you is isolation between implementation and interface. Usually, that's just not a very high priority.

jalf
+1  A: 

pImpl is very useful when you come to implement std::swap and operator= with the strong exception guarantee. I'm inclined to say that if your class supports either of those, and has more than one non-trivial field, then it's usually no longer down to preference.

Otherwise, it's about how tightly you want clients to be bound to the implementation via the header file. If binary-incompatible changes aren't a problem, then you might not benefit much in maintainability, although if compile speed becomes an issue there are usually savings there.

The performance costs probably have more to do with loss of inlining than they do with indirection, but that's a wild guess.

You can always add pImpl later, and declare that from this day forth clients will not have to recompile just because you added a private field.

So none of this suggests an all-or-nothing approach. You can selectively do it for the classes where it gives you benefit, not for the ones it doesn't, and change your mind later. Implementing for example iterators as pImpl sounds like Too Much Design...

Steve Jessop
+2  A: 

I use the idiom in a couple of places in my own libraries, in both cases to cleanly split the interface from tthe implementation. I have, for example, an XML reader class fully declared in a .h file, which has a PIMPL to a RealXMLReader class which is declared & defined in non-public .h and .cpp files. The RealXMlReader in turn is a convenience wrapper for the XML parser I use (currently Expat).

This arrangement allows me to change from Expat in the future to another XML parser without having to recompile all the client code (I still need to re-link of course).

Note that I don't do this for compile-time performance reasons, only for conveniance. There are a few PIMPL fabnatics who insist that any project containing more than three files will be uncompilable unless you use PIMPLs throughout. It's noticeable that these people never produce any actual evidence, but only make vague references to "Latkos" and "exponential time".

anon
+13  A: 

One of the biggest uses of pimpl ideom is the creation of stable C++ ABI. Almost every Qt class uses "D" pointer that is kind of pimpl. This allows performing much easier changes withot breaking ABI.

Artyom
Excellent point. I'd go as far as using C functions and explicitly passing the pimpl pointer as the first parameter to each method call instead of C++. C (preferably through dynamic libraries) is much more compatible between different compilers and environments than C++.
Laserallan
+2  A: 

Code Clarity

Code clarity is very subjective, but in my opinion a header that has a single data-member is much more readable than a header with many data-members. The implementation file however is noisier, so clarity is reduced there. That might not be an issue if the class is a base class, mostly used by derived classes rather than maintained.

Maintainability

For maintainability of the pimpl'd class I personally find the extra dereference in each access of a data-member tedious. Accessors can't help if the data is purely private because then you shouldn't expose an accessor or mutator for it anyway, and you're stuck with constantly dereferencing the pimpl.

For maintainability of derived classes I find the idiom is a pure win in all cases, because the header file lists fewer irrelevant details. Compile time is also improved for all client compilation units.

Performance

Performance loss is small in many cases and significant in few. In the long-run it is in the order of magnitude of virtual functions' performance loss. We're talking about an extra dereference per access per data-member, plus dynamic memory allocation for the pimpl, plus release of the memory on destruction. If the pimpl'd class doesn't access its data-members often, the pimpl'd class' objects are created often and are short-lived then dynamic allocation can out-weigh the extra-dereferences.

Decision

I think classes in which performance is crucial, such that one extra dereference or memory allocation makes a significant difference, shouldn't use the pimpl no matter what. Base classe in which this reduction in performance is insignificant and of which the header file is widely #include'd probably should use the pimpl if compilation time is improved significantly. If compilation time isn't reduced it's down to your code-clarity taste.

For all other cases it's purely a matter of taste. Try it and measure runtime performance and compile-time performance before you make a decision.

wilhelmtell