Question

Is there any reason for the implementation class as used in the pimpl idiom to have any private members at all? The only reason I can really think of is to protect yourself from yourself -- i.e. the private members serve to enforce some kind of contract between the class and the user, and in this case the class and the user are rather intimately related, so it seems unnecessary.

Was it helpful?

Solution

I think people are confusing the Pimpl idiom with Adapter/Bridge/Strategy patterns. Idioms are specific to a language. Patterns can apply to many languages.

The Pimpl idiom was devised to address the following problem in C++: Private members of a class are visible in the class declaration, which adds unnecessary #include dependencies to the user of the class. This idiom is also known as compiler firewall.

If the implementation is written directly in the outer class's corresponding *.cpp file, and is not accessible outside the module, then I think its perfectly fine to simply use a struct for the Pimpl class. To further re-enforce the idea that implementations are not meant to be directly re-used, I define them as a private inner struct:

// foo.h
class Foo : boost::noncopyable
{
public:
   ...

private:
   struct Impl;
   boost::scoped_ptr<Impl> impl_;
};

// foo.cpp
struct Foo::Impl
{
   // Impl method and member definitions
};

// Foo method definitions

As soon as there's a header file for the implementation class, I think we are no longer talking about the Pimpl idiom. We are rather talking about Adapter, Bridge, Strategy, interface classes, etc...

Just my 2 cents.

OTHER TIPS

Depends on your implementation of pImpl - specifically where you enforce the class invariant, but in general I see no need for the impl part to have protected/private members. In fact, I usually declare it as a struct.

In theory a pimpl class is still just a class like any other. That it is a concrete implementation of an interface doesn't mean that other code isn't a client of the pimpl class itself.

That said, in practice I have found that pimpl classes tend to be much closer to structs with some member functions rather than full fledged objects, and have less need to separate the interface from the implementation.

Why shouldn't it have private members? Just because you're defining one interface as PIMPL doesn't mean that you will at no other time want to use the class.

It's still a class. Data should probably be private or protected. Operations on data that will never be accessible to the public, private or protected. Operations that you might wish to expose, protected, or public.

The only reason I can really think of is to protect yourself from yourself

Which is why "private" and "protected" are there in the first place. Of course you should use them in your implementation - the only time I would not is if the implementation has no behaviour (in which case it isn't really an implementation).

(I misunderstood the question, so I'm changing my answer.)

The implementing class, which is pointed to by the class with the pimpl, should be a regular class, with just as much reason to hide private details as any other. In fact, it's often a pre-existing class, with the pimpl layer added later to break dependencies and perhaps simplify the interface a bit.

Because private means better data encapsulation.

It seems silly, I know, but we have a way of defining interfaces at work that is very simple:

class Interface;

class Concrete { public: .... private: Interface* m_interface; }

class ConcreteFoo: public Interface {};

The main advantage: you can swap for another ConcreteBar at any moment. It is, in fact, a combination of Pimpl and Strategy pattern, and the constructor of Concrete will call a Factory that will be responsible to serve the effective object.

If you cannot think of a second way of implementing the heart, just encapsulate it in a class. This way if you later have to refactor, you'll just have to compose an abstract Interface with the exact same set of methods, change a few pointers (and the constructor) and you'll be all right ;)

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top