On Tue, 25 Nov 2008 20:03:22 +0100, "Rutger ter Borg" <rutger@terborg.net> said: < snip >
Herewith my non-exhaustive review:
I've looked at the library, good work, it looks very interesting. I did a quick reading, and in particular, I paid attention to the many of its intended uses (in the Rationale), and how this would have to be done in practice (the Interface).
* The design notes state that it is based on X.667-E. This document describes three construction mechanisms: time-based, rng-based, and string-based. The proposal doesn't have the time-based constructor. Is there a reason why this is missing?
Only because I'm not sure how to implement the time-based construction mechanism a portable way.
* Is the boost namespace that cluttered that it isn't possible to hold "boost::uuid" in it? I think boost::uuids::uuid is kind of repetitive. (Or is there a policy against putting stuff in the boost root-namespace?)
I did this because of http://www.boost.org/development/requirements.html#Naming_consistency But I have no problem doing something else if this list wants me too.
* Construction is described in "Constructors" and "Construction" -- this could perhaps lead to confusion?
Do you mean "Constructors" and "Creation"? Hmm, I'll give this some thought.
* In Representation: Is there a reason for a .to_string() member function instead of an operator std::string()? I.e., it reduces std::string s = u.to_string() to std::string s( u );
I tend to avoid implicit conversions. One can also use boost::lexical_cast<std::string>(uuid). Again, if this list wants operator std::string() and the like, I will add it.
* Considering the examples in the Rationale and the available constructors: I'm curious how to easily "tag an object" using the provided constructors. By a random number? Or could I just pass the address of the object and use a string-based method? In other words, it would be nice if such an example from the Rationale would be topic of an example.
I think an example for this is a good idea. On way would be: class Foo { public: Foo() // create a temporary generator (or pass one in the constructor // or create a singleton) and initialize m_uuid with it. : m_uuid(boost::uuids::uuid_generator()()) {} private: const boost::uuids::uuid m_uuid; };
I vote for acceptance into boost, given that just a bit more attention is paid to a potential user's convenience.
Kind regards,
Rutger
Thanks, Andy.