Subject: Re: [boost] pimpl library proposal
From: Krzysztof Czainski (1czajnik_at_[hidden])
Date: 2010-12-14 11:09:56
2010/12/14 Vladimir Batov <vbatov_at_[hidden]>
> I see. But if I understand correctly, for each smart pointer type, there
>> needs to be a specialization of pimpl_base written from scratch.
>>
>
> I am not sure where you've got that impression from. There is one
> pimpl_base implementation. Pointer- or value-semantics behavior are defined
> by the provided policy class. Pls see the declaration above.
You are right, I misunderstood the definition of pimpl_base. I thought it
was being specialized outside of pimpl, but it is not the case.
template<class T>
struct pimpl /// Generalization of the Pimpl idiom
{
struct implementation;
template<class> class impl_ptr;
template<template<class> class> class pimpl_base;
/// Convenience typedef to deploy pimpl with value semantics.
typedef pimpl_base<impl_ptr> value_semantics;
/// Convenience typedef to deploy pimpl with pointer semantics.
typedef pimpl_base<boost::shared_ptr> pointer_semantics;
};
So pimpl_base can accept as its template parameter only a template that
takes one argument, like shared_ptr. But it cannot take a template with two
or more parameters, correct?
Why not just do the following?
template < class SmartPtr > class pimpl_base;
Another question: what is the purpose of Your pimpl struct? In other words,
assuming pimpl_base takes a regular template parameter, what is the
advantage of
class MyClass : pimpl<MyClass>::pointer_semantics;
over
class MyClass : pimpl_base< shared_ptr<MyClassImpl> >;
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...
>
I do not immediately see value in Pimpl accepting "any smart pointer" which
> are policy classes for Pimpl. In fact, I do not know of any policy-based
> class which would accept *any* other class as its policy. I am convinced a
> class to be used as a policy by another class must conform to a certain
> interface.
I admit "any" is not the right word for what I had in mind. I ment some
smart pointer, that the author of the pimpl<> template does not know of;
that smart pointer may not yet exist. Since You place typedefs inside pimpl
(value_semantics, pointer_semantics), I presume You intend to predict all
possible used spart pointers. Otherwise usage would be something like:
class MyClass : pimpl<MyClass>::pimpl_base<MySmartPtr>;
whereas MySmartPtr would have to be a template with one template argument.
...
>
>> 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.
>
I am under impression that the functionality you are suggesting is already
> available.
I can't find it
> Namely, (unless I am still missing your point) "make_shared" quality is
> provided by boost::shared_ptr itself and nothing more needs to be done.
I disagree. Make_shared is provided separatly from shared_ptr, and must be
used instead of operator new, when creating an object.
> As for "custom allocator/deleter" or any other creation policy, then that
> functionality belongs in the "implementation" class and deployed via Pimpl
> interface. So, it seems nothing more needs to be done in Pimpl either.
A constructor from pimpl_base:
pimpl_base() : impl_(new implementation()) {}
It creates implementation using operator new. I was suggesting adding a
factory parameter to pimpl, and delegating creating to the factory, instead
of explicitly calling new implementation()
>
> 3. The user should have no way of telling if MyClass uses pimpl or not. It
>> should be totally transparent.
>>
>
> I happen to disagree. Pimpl implements a certain pattern/idiom which has
> certain qualities. The user deploying that pattern/idiom expects those
> qualities. After all, that's why he/she is deploying that pattern/idiom in
> the first place. I do not immediately see benefits of hiding those
> qualities.
I think we don't understand each other on the definigion of "user". Earlier
I proposed to call a user someone who uses MyClass, and to call an
implementer someone, who implements MyClass and optionally uses pimpl. When
You say "user deploying [pimpl] pattern/idiom", You must mean the
implementer.
However, if You don't think hiding the use of pimpl from the user is a good
thing, then I guess we just have different oppinions about it.
Well, I understand You expose the operator bool_type for example.
>
> Again, I believe it is part of the idiom. For example, boost::shared_ptr is
> similar in that regard.
>
I see, so we disagree on this. I don't think exposing operator bool_type is
part of the idiom. I think we did not agree on the definition of pimpl, and
I think this leads to all the other things we disagree upon ;-)
[...]
I am sure your Pimpl implementation does everything just the way you want
> it. :-) And if that works better for you than an already available
> component, then I do not see anything wrong with deploying it. I do the same
> all the time. ;-) Still, I suspect if you try deploying your no-frills
> version of a larger scale, it'll start looking more and more as the proposed
> Pimpl. There are countless examples of that behavioral pattern (Java comes
> to mind). As for the proposed Pimpl, let me assure you that the
> functionality provided and described there is there for a reason. May I
> suggest you have another look at the documentation for the proposed Pimpl?
> You might come to appreciate its versatility... or might not. :-)
>
> Good luck and all the best in your endeavors,
Good luck and all the best to You too,
Kris