
"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