Boost logo

Boost :

From: Pavel Vozenilek (pavel_vozenilek_at_[hidden])
Date: 2005-05-10 06:17:30


"Dan Rosen" wrote:

Thanks for your review. It almost looked here as if no one cares.

> I think this
> particular implementation has some novel ideas, particularly
> "multiton," which I know I've had to implement before, but have never
> seen anybody codify as a pattern per se.
>
There is Ruby library (in permanent beta) implementing multiton.
http://raa.ruby-lang.org/project/multiton/

> 1. Creator policy is redundant with the standard Allocator concept, I
> think. Though the standard Allocator concept has some subtle, tricky
> semantics, I think it's nevertheless worth using: that would permit
> interoperability with existing allocator implementations.
>
Creators allows to use arguments when singleton is (re)created, like:

Example::pointer ptr;
....
ptr->create(111, "abc");

....
ptr->destroy();
ptr->create();
....

While it would be possible to simulate this feature with copy constructor
it would violate common expectations placed on singleton class.

> 2. The auto_reset struct has some conceptual issues. The biggest of
> these is that the scope of the auto_reset object, if I understand it
> correctly, can exceed the lifetime of the variable it references. If
> such an object is strictly necessary, I believe it should own the
> object it references. Another minor nit: the template should be
> parametrized not only on the variable type, but also the reset value
> type. And lastly, the documentation contains implementation details in
> the interface.
>
auto_reset.hpp is simplified version of <boost/state_saver.hpp>.
There were discussions on mail-list about this utility.

Using <boost/state_saver.hpp> would be prefereble to home grown
variant, IMHO.

In singleton library auto_reset is used internally w/o dangers
you mention. The documentation should be hidden, indeed.

> 3. The leaky_lifetime class is indicated as "not recommended for
> general use." I tend to disagree: in production code I've had
> experience with, it's been very appropriate to manually control the
> order of destruction of singleton resources, because there are often
> dependencies. Some of these issues can be mitigated by
> dependency_lifetime, which is a good contribution. But I think the use
> of leaky_lifetime shouldn't be explicitly discouraged. I think it
> would be more appropriate to name the class simply
> "unmanaged_lifetime".
>
Dependencies between singletons are best solved by
owning other singleton pointer. This is documented at the end
of tutorial (basic.htm) but obviously it should be more visible.

"unmanaged" sounds as something related to .NET.
I had suggested "immortal" but native English speakers
are better to think about this.

> 4. I think multiton is a good idea, but its execution is incomplete.
> If it's in the library, my feeling is that it should implement the
> requirements of the standard Pair Associative Container fully. (I'd
> also suggest calling it singleton_map).
>
More features to multiton has been planned with so called
"iterable_multiton". This one would be more like std::map
and in addition, user will be able to iterate through it by creation
time for individual objects.

> 5. The polymorphic_pointer and polymorphic_reference helpers seem
> sketchy to me somehow. I don't understand their use. I'd like to see
> use cases in the documentation.
>
Yes.

> 6. The exists(), is_null() and operator bool() methods fall prey to a
> well known C++ pitfall. Have a look at Bjorn Karlsson's article, "The
> Safe Bool Idiom": http://www.artima.com/cppsource/safebool.html.
>
Looks so. It would be nice to have a common support for this
in Boost, though.

> 8. The docs are sorely lacking in detail. For example, the methods in
> the basic_singleton and singleton_ex interfaces are not enumerated
> anywhere, and none of the examples I saw show how to get an instance
> of a singleton. All that I felt was illustrated were various
> regurgitations of the "curiously recurring template pattern."
>
Expansion of the documentation and more examples is planned.

While having complete and perfect documentation at this point
would be ideal there are real world constraints. I watched
the documentation go steadily better and have reason to think
it will do so in the future.

/Pavel


Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk