2010/12/9 Vladimir Batov <vbatov@people.net.au>
Kris,
Thanks for your input. It's much appreciated.
Have you had a chance to read through the Pimpl documentation? Currently, the described Pimpl provides two policies -- pointer_semantics and value_semantics. In the pimpl.hpp code it looks as follows:
Yes, I have read that documentation ;-)
template<class T> struct pimpl { typedef pimpl_base<impl_ptr> value_semantics; typedef pimpl_base<boost::shared_ptr> pointer_semantics; };
More so, nothing is stopping us from extending it with, say, COW semantics:
template<class T> struct pimpl { typedef pimpl_base<impl_ptr> value_semantics; typedef pimpl_base<boost::shared_ptr> pointer_semantics; typedef pimpl_base<cow_ptr> cow_semantics; };
Isn't it "customization of the copying policy"? Or did you mean something else?
I see. But if I understand correctly, for each smart pointer type, there needs to be a specialization of pimpl_base written from scratch. Assuming that a SmartPtr provides pointer operators and performs copying according to some policy, You might consider writing a general pimpl_base<SmartPtr>. Now suppose pimpl_base<SmartPtr> does accept any smart pointer: scoped_ptr, shared_ptr, clone_ptr, cow_ptr, etc... This is where the creation policy is needed. See below... And perhaps a way of customization of the creating policy, a factory
of some sort...
I am under impression that I do not quite understand what you mean by "customization of the creating policy" either. Isn't that quality achieved by overloaded constructors? If it is so, then the developer provides the set of overloaded constructors for his/her class. Regardsless of those constructors Pimpl does not need to be modified.
The default creation policy would be to create impl on the heap using operator new, and passing the resulting pointer to SmartPtr. But for example, when using shared_ptr, it might be a good idea to use a) make_shared instead of operator new, b) a custom allocator/deleter. Thus, I suggest making pimpl_base< SmartPtr, Factory = HeapFactory<SmartPtr>
a general class with no specializations per SmartPtr.
3. The user should have no way of telling if MyClass uses pimpl or not. It
should be totally transparent.
I find it hard to visualize what you have in mind. Would it be possible to
provide an example where you believe manually-written pimpl-idiom class is more transparent than a class derived from the proposed pimpl?
Well, I understand You expose the operator bool_type for example. But that is not all. I believe a user is also someone deriving form MyClass. Your pimpl takes part in runtime polymprphism, and for that it mus be visible by derived classes.
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.
Well, you omitted concrete examples/explanations clarifying why you prefer composition over inheritance in this particular case. Consequently, I have that nagging feeling that you are taking the "prefer composition over inheritance" a tiny bit too dogmatically. From where I stand I feel that Pimpl-derived class needs/wants to expose the complete interface of Pimpl (after all that's why those methods were introduced in Pimpl -- to re-use them). That indicates inheritance.
In my view Liskov Substitution Principle is a perfect litmus test for "should I be inheriting from this type?". And in my view Pimpl fits the bill.
I just thought that, since the user should not know of pimpl's existence, there should be no functions to inherit from it. After all, functional inheritance (as opposed to architectural inheritance)
is quite common programming technique.
As for your concern about "a lot of operations" in Pimpl "potentially conflicting with operators" of YourClass, then my reading of the specification is somewhat different. Namely, as I understand, a method introduced in the derived class makes *all* the methods with the same name (not just the same signature) in the base class inaccessible.
True. So, what if the implementer adds operators * and -> to MyClass? Accessing pimpl is still possible, but not as convenient any more. satic_cast<implementation&>(*this)->... or an extra local reference is always needed: void MyClass::f() { implementation& impl = *this; impl->... }
Namely, I do not see why a pointer_semantics-based Pimpl should not provide op==() automatically (as shared_ptr does). On the other hand, the user has to provide op==() for a value_semantics-based Pimpl.
For the pointer_semantics-based Pimpl: I do not think that MyClass should have pointer semantics just because its pimpl has pointer semantics. So I am against forwarding op== from the underlying shared_ptr. For the value_semantics-based Pimpl: If the implementer wants op== in MyClass, he should provide it. And impl should only contain data, and perhaps helper functions, that would otherwise be private nonvirtual member functions of MyClass without pimpl. Or it should not implement the possibility of a null object -
boost::optional is for that.
Well, my view on the usefullness of pimpl::null() happens to be quite
different. In fact, that pimpl::null is there because I use it a *lot*. Pimpl belongs to a smart-pointer-pattern family and shared_ptr and the likes provide such functionality (in a somewhat obscure form of shared_ptr() IMHO of cource).
On second thought, I agree, null() might come in handy, as long as there is a way to disable it if it isn't needed. Well, "pimpl should have nothing to do with polymorphism" is quite a
determined statement. ;-) That mentioned functionality has its purpose -- it implements Non-Virtual Interface Idiom. If you have no need for that, that's OK. We should not be denying it to others, should we? In fact, I happen to use it to fight the complexities of architectural inheritance (where it's unavoidable).
I just think virtual functions should just stay in MyClass, and not go into pimpl, because they should not be hidden. That is why I think "pimpl should have nothing to do with polymorphism" ;-) Thanks for the interesting discussion, Vladimir ;-) For an example of how I use pimpl, and my pimpl helper, which I called value_ptr, I shall quote my earlier post, adding an example of how some member function of MyClass might be implemented in terms of pimpl: MyClass.h: class MyClass { public: MyClass(); // needed, but trivial ~MyClass(); // sometimes needed, but trivial void f(); private: struct Impl; value_ptr< scoped_ptr<Impl> > impl_; }; MyClass.cpp: struct MyClass::Impl { /*...*/ void f(int) {} }; MyClass::MyClass() {} // value_ptr automatically allocates an Impl instance MyClass::~MyClass() {} // value_ptr automatically deallocates Impl void MyClass::f() { if ( cond ) impl_->f(1); else impl_->f(10); } The source code for value_ptr together with a few other related tools can be found here: https://libcz.svn.sourceforge.net/svnroot/libcz/trunk/boost Regards, Kris