Boost logo

Boost Users :

From: Nat Goodspeed (nat_at_[hidden])
Date: 2008-01-15 14:15:14


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:
   - singleton, mutexed_singleton and thread_specific_singleton
   - bypassing the requirement for friend declarations with the ctor
taking boost::restricted
   - 'instance' as a static data member rather than a static member function
   - the lease mechanism
   - control of destruction order

   My one additional comment on design: people are very used to writing
my_singleton::instance()->member. I like the coolness of writing
my_singleton::instance->member instead, but I suspect I'll hear some
grumbling from colleagues who keep getting reminded by compile errors. I
wish it were possible to provide 'instance' as both a static data member
and a static member function...
   But I'd be good with having this become the new convention.

> * What is your evaluation of the implementation?

   I didn't review the implementation.

> * 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().
   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.
   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.
   Very little is said about BOOST_SINGLETON_PLACEMENT and
BOOST_SINGLETON_PLACEMENT_DECLARATION. The reference material mentions
them without any explanation.
   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? 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.

> * What is your evaluation of the potential usefulness of the library?

   Very. "If it didn't exist, we would have to invent it." ;-)

> * Did you try to use the library? With what compiler?
> Did you have any problems?

   I didn't try it.

> * How much effort did you put into your evaluation?
> A glance? A quick reading? In-depth study?

   I read the documentation and cross-checked a couple of interface
points (SubsystemTag) in the source code.

> * Are you knowledgeable about the problem domain?

   Yes, I've implemented the Singleton pattern a few times now.

> And finally, every review should answer this question:
>
> * Do you think the library should be accepted as a Boost library?
> Be sure to say this explicitly so that your other comments
> don't obscure your overall opinion.

   Yes! Thumbs up! Once the documentation is updated as mentioned above,
this will be an excellent and much-needed addition to Boost.


Boost-users list run by williamkempf at hotmail.com, kalb at libertysoft.com, bjorn.karlsson at readsoft.com, gregod at cs.rpi.edu, wekempf at cox.net