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

From: Anthony Williams (anthony_w.geo_at_[hidden])
Date: 2008-01-16 05:52:22

Tobias Schwinger <tschwinger_at_[hidden]> writes:

> thanks for your review.

You're welcome.

> Anthony Williams wrote:
>> I don't think this is at all useful. People should not be encouraged to use
>> singletons, as there is already a tendency towards overuse. In most
>> circumstances, singletons are a poor design choice, so there is little
>> requirement for their use.
>
> Any design that's picked carelessly is poor or just at best a fluke.

Can't argue with that!

> Having a framework internally use some Singletons can greatly simplify
> its use.

Possibly, but I don't think they're needed even then. If they're an
implementation detail of the framework, you don't need to make things a
singleton in order to ensure there is only one instance --- just create one.

> Exposing a singleton to a user provides more flexibility than
> exposing a static interface (and can also improve performance).

I don't see how. You can easily rewrite a static interface to use a singleton
internally. Having a framework provide the user with a (reference-counted)
pointer to an interface suitably hides all the details.

> A "tendency towards overuse" is not a good reason to reject a library,
> as it won't stop overuse and encourages more half-baked solutions that
> are written in a hurry.

It is better to educate people in better ways of doing things (and provide
tools to make those things easy) than enable them to easily do something
that's generally a bad idea.

>>> * What is your evaluation of the design?
>>> * What is your evaluation of the implementation?
>> 
>> The design mixes several independent issues --- ensuring there is only one
>> instance of a class and avoiding initialization order problems with on-demand
>> initialization for starters.
>> 
>> A simple wrapper class that allows for on-demand initialization, would be
>> useful. Conflating that with "there shall be at most one instance" is not.
>> 
>> Again, allowing for a preferred destruction sequence of objects with such
>> on-demand initialization might also be useful, but does not belong with the
>> one-instance constraint.
>
> What is the point in managing construction order without static context?

Sometimes there are uses for globals. In those cases, it is important to
manage the construction and destruction order, independent of how many
instances of any given class there may be.

> What is the point of having more than one instance of a class that lives
> in static context -- and how would it be captured syntacticly?

I can imagine a use for having lots of instances of boost::mutex at global
scope --- each mutex protects a different thing. You don't need to capture the
syntactic idea "there may be more than one of this class" --- that is the base
level assumption.

>> thread_specific_singleton is overly complex for what should just amount to a
>> simple use of thread_specific_ptr.
>
> It uses 'thread_specific_ptr' and has additional responsibilities.
>
> Could you be more specific, please?

thread_specific_ptr on its own provides for one instance of an object per
thread. I disagree that a class should impose that there is one and only one
instance per thread (and no globals) --- this is a decision for the user of a
class, not for the class itself. As I understand from the docs, the additional
responsibility is the destruction order management. I agree this is a useful
facility to provide, and think it is independent of the singleton nature of
the classes, so should be separated into its own class.

>> I've written code that used singletons (and regretted it later), and used
>> code that other people have written containing singletons.
>
> So you should like this library: The design chosen allows you to easily
> substitute Singletons with Smart Pointers.

I disagree. If people know something is a singleton, then they don't bother
keep the reference around, as they can get it again later. Changing code that
does that to code that can use a passed in instance requires adding parameters
and/or storing references. You can't just change uses of a singleton to use a
smart pointer instead without thought to *how* it's being used.

Anthony
-- 
Anthony Williams
Just Software Solutions Ltd - http://www.justsoftwaresolutions.co.uk
Registered in England, Company Number 5478976.
Registered Office: 15 Carrallack Mews, St Just, Cornwall, TR19 7UL