On Fri, Dec 12, 2008 at 11:44, Andy Tompkins <atompkins@fastmail.fm> wrote:
Here is what I see the interface looking like based on feedback from the review so far. It would, of course, be split into different header files.
I like it.
// no default constructor private: uuid();
Great in theory, but I think it'll break lexical_cast<uuid>("21f7f8de-8051-5b89-8680-0195ef798b6a"), since the lexical_cast wants to default-construct the target type into which it will read.
// valid expression // generator() - result type must be convertible to a uuid template <typename Generator> explicit uuid(Generator & generator);
Does this use the Concept defined at the end? Also, when is uuid(gen) significantly more convenient than uuid(gen())? I'd have thought that the copy constructor was plenty.
// assert(std::distance(begin, end) >= 16); template <typename ByteInputIterator> uuid(ByteInputIterator begin, ByteInputIterator end);
Glad to see the exception is gone.
bool is_nil() const;
operator unspecified_bool_type() const; // return !is_nil();
shared_ptr.hpp says // operator! is redundant, but some compilers need it so providing it might be good; I don't know.
// removed get/set_showbraces (does anybody want/use them?) template <typename ch, typename char_traits> std::basic_ostream<ch, char_traits>& operator<<(std::basic_ostream<ch, char_traits>& os, uuid const& u); template <typename ch, typename char_traits> std::basic_istream<ch, char_traits>& operator>>(std::basic_istream<ch, char_traits>& os, uuid& u);
So long as reading still can handle them, it's probably easy enough to stream the brackets explicitly if desired, so that seems a fine decision. (The statics in headers always makes be nervous.)
class nil_generator // always generates a nil uuid { public: typedef uuid result_type; uuid operator()(); }; uuid nil(); // easy to use function - should it be included?
Might as well include it, I guess. On that note, are you including the name-based UUID name spaces anywhere?
// I guess technically a transformer since it does not have a zero argument // operator() class name_based_generator // uses sha1 hash {
On the assumption that an MD5 version would be a reasonable extension, what about templating this on the hasher, defaulting to the sha1 one? That way both the defaults would have the <>, for consistency (random_generator<> and name_based_generator<>). Speaking of which, the version_type enum entry and the generator for random UUIDs don't really match, since the former has number_based (paralleling the name_based enum entry and generator) but the latter does not. random_number_based_generator is admittedly getting rather unwieldy, though.
uuid operator()(const wchar_t* name); // use std::wcslen
What happens here if there are wchar_ts in the stream outside of 0xFF?
class windows_generator // uses UuidCreate class linux_generator // used uuid_generate typedef ... native_generator;
Should these be parametrized at all? Or do people just want defaults, if they want platform ones?
using uuids::uuid; // so nobody has to type boost::uuids::uuid
Good.
/* uuid Generator concept
Notation: X - a type that is a model of uuid generator x - an object of type X
Associated Types: Result Type - X::result_type - this must be convertable to boost::uuids::uuid
Valid Expressions: x() - return a boost::uuids::uuid */
If result_type allows convertible, then the x() should probably return a result_type, not strictly a uuid. What's this used for? Would it worth be adding one for message UUID generators too? Thanks for your work on this, ~ Scott