
2010/12/7 Vladimir Batov <vbatov@people.net.au>
See if the pimpl proposal currently sitting in the review queue
Hello Vladimir, I took a look at Your pimpl code. I'm afraid it doesn't look like something I'm looking for. Here is a list of my expectations of pimpl. Suppose the implementer is preparing a class MyClass. A user is someone using MyClass. 1. Pimpl should hide the implementation. Your pimpl does that well. 2. Pimpl should automate construction and destruction of the Impl struct. Your pimpl does that, but lacks a way of customization of the copying policy. And perhaps a way of customization of the creating policy, a factory of some sort... 3. The user should have no way of telling if MyClass uses pimpl or not. It should be totally transparent. With Your pimpl, MyClass inherits from pimpl_base, bringing in a lot of operations: operator*(), operator bool_type() and others. They are protected, but they still are there - potentially conflicting with operators, that MyClass might be providing for the user. Therefore I'd prefere making pimpl a member over inheriting from it. 4. Customizability of pimpl: the implementer might want one of different approaches to copying: a) disallow, b) share, c) clone, d) others like COW. I'm not sure which of these Your pimpl provides. I would recommend a template parameter to choose a policy. It might be just a smart pointer owning the Impl object. 5. Pimpl should not do more, then there is to the pimpl idiom. That is, it should hide the implementation. But it should not implement features like operator !=, when the implementer might not want those. Library operators is for that. Or it should not implement the possibility of a null object - boost::optional is for that. 6. You write about pimpl and dynamic polymorphism. I think pimpl should have nothing to do with polymorphism. Any polymorphic part of a class, even a private virtual function, isn't truly private, so it shouldn't be in pimpl. And everything that is in a pimpl should be truly hidden, even from the derived classes. Te derived class should not be able to tell if MyClass even uses pimpl or not. And the derived class might as well have it's own pimpl. See if the pimpl proposal currently sitting in the review queue meets your
needs. If not, let's extend it.
Best, V.
Well, in my opinion, the pimpl You propose does too much things, that I wouldn't want in it. They might be useful sometimes, but not always. But that opinion might be based on the fact, that I understand the pimpl idiom differently. I use the approach similar to Herb Sutter's: http://www.gotw.ca/gotw/024.htm I think Your pimpl lacks a good enough way to customize the copying and creation policies, but I suppose You wouldn't have anything against adding that... Regards, Kris