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

From: Tobias Schwinger (tschwinger_at_[hidden])
Date: 2008-01-17 03:14:48

Gennadiy,

thanks for your through and through negative review.

Gennadiy Rozental wrote:
> "John Torjo" <john.groups_at_[hidden]> wrote in message 
> news:478B978A.6090503_at_torjo.com...
>> Hi all,
> 
>> * What is your evaluation of the design?
> 
> In short: in places too complicated, in places plain wrong.
> 
> In details:
> 
> 1. mutexed_singleton
> 
> This should be singleton<synchronized<T> >. Current design mixes two 
> orthogonal concepts: uniqueness of class instance and class methods 
> synchronization and this is obviously wrong.
> 
> 2. thread_specific_singleton.
> 
> The same as above. This should be singleton<thread_specific<T> >. Current 
> design mixes two orthogonal concepts: uniqueness of class instance and 
> thread specific object storage.

One could implement what you propose. It would probably even make things 
easier on my side.

It complicates client code and compromises the replaceability of all the 
Singleton templates, which is a mistake.

> 
> 3. public construction and boost::restricted
> 
> I don't like this solution and very much prefer private constructor with 
> friend declaration (especially if encapsulated into single macro). This will 
> be a never ending source of confusion (what is boost::restricted, can I have 
> restricted access, how do I create restricted instances etc) and doesn't 
> really protect - once can still create instances of my class with valid C++ 
> code.

You're still free to use a private constructor and a friend declaration.

> 
> 4. DLL support
> 
> Explanation of DLL issues in docs is very unclear. No examples provided. No 
> tests showing that solution actually works. Accordingly I can't really 
> comment on it, but from what I got I really doubt it.

The test just hasn't been automated yet. And, yes, the documentation can 
use some improvement.

> 
> 5. instance proxy.
> 
> Given complete absence of rationale of this design, I can't really devise 
> why is was chosen, but it seems wrong. We end up with pricy complication 
> with unclear purposes. What's wrong with just returning T& from instance 
> method? The same time we can get rid of all this member_dereference staff 
> and it's limitations.

What limitations are you talking about?
The ability to have automatic locking when using a member pointer?

> 
> 6. lease.
> 
> Pretty much the same as above. synchronized<T> should have it's own lock 
> facility. Otherwise instance() should be straightforward access to the 
> instance and no need for any proxies/leases.

Again, fails to see the advantage of a uniform interface.

> 
> 7. MT support
> 
> Setting aside mutexed and thread specific singletons covered above, regular 
> singleton handling of MT is wrong as well. In simple words my primary 
> complain: you can't use define based solution for switching MT/non MT. It 
> should be template parameter. As of now there is no singleton that can work 
> in presence of threads but without disregard to them.

I can't get your point. There probably is none.

> 
> 8. No Meyer's singleton
> 
> What if do not want all this manager staff?

What's "all this manager staff" and what's a "Meyers' singleton"?

Regards,
Tobias