Re: [boost] [review] Fast track review of Boost.Utility/Singleton

From: Tobias Schwinger (tschwinger_at_[hidden])
Date: 2008-01-16 03:06:37

Nat,

thanks for your very positive review.

Nat Goodspeed wrote:
> John Torjo wrote:
> 
>> Today starts the formal Fast-Track review of the Boost.Utility/Singleton 
>> library.
>>
>> What to include in Review Comments
>> ==================================
>>
>> Here are some questions you might want to answer in your review:
>>
>> * What is your evaluation of the design?
> 
>    Very strong. I like it!
>    I've implemented CRTP Singleton templates a few times before -- 
> highlighting the need for a Boost library. ;-) I'm pleased with the 
> deluxe functionality:

>> * What is your evaluation of the documentation?
> 
>    Very clear, though not entirely complete, as described below. The 
> organization and style is excellent, and when polished, this 
> documentation will be exemplary. I like the examples. Pointing out the 
> ability to bind() instance and/or lease, and the semantics of each, is 
> great. Also the multiple-inheritance example to graft Singleton behavior 
> onto an existing class.
>    I didn't fully understand the thrust of the example described as: "We 
> might as well have a static facility use the Singleton internally, by 
> using non-public inheritance". A few more words about that example would 
> be useful; maybe a bit of the implementation of static void message().

I might have kept it so brief since I'm not too sure it's good practice.
I think I'll better just remove it.

>    It appears that the third template parameter SubsystemTag was a 
> recent introduction. The documentation (especially the reference 
> material) doesn't describe it enough, and often fails to mention it 
> entirely.

Yes.

>    The first paragraph under
> http://torjo.com/tobias/index.html#boost_utility_singleton._singletons_and_dynamic_libraries
> seems to want to be a bullet list that got flowed instead.

Yes.

>    Very little is said about BOOST_SINGLETON_PLACEMENT and 
> BOOST_SINGLETON_PLACEMENT_DECLARATION. The reference material mentions 
> them without any explanation.

That "Singletons and dynamic libraries" section explains their purpose.

What exactly is missing?

I'm not too fond of specifying what those macros expand to and I don't
really want to mention them in the introductory section either, as for
me they are kind-of workarounds for the imperfect ABIs we have to deal
with.

>    The example
> http://torjo.com/tobias/index.html#boost_utility_singleton._singletons_and_dynamic_libraries.example_for_a_portable_singleton_exposed_by_a_dynamic_library
> appears to be inconsistent: shouldn't mylib_tag and library_tag be the 
> same, one way or the other? 

They should. My head must had been on vacation revising the docs.

> Also, that example combines three different 
> mechanisms: BOOST_SINGLETON_PLACEMENT, destroy_singletons() and 
> SubsystemTag. More explanation of the role of each (why are we using 
> that mechanism here?), and how they interact -- or remain orthogonal -- 
> would clarify.

Yes.

Regards,
Tobias