On Fri, 12 Dec 2008 15:42:32 -0500, "Scott McMurray" <me22.ca+boost@gmail.com> said:
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.
Your right! Hmm. I want to be able to use lexical_cast as above, but I also don't want a default constructor. I think I will optionally include/exclude the default constructor with a #define. And I think a more explicit way to convert a string representation of a uuid to a uuid should be included. Something like the following: template <typename ch, typename char_traits, typename allocator> uuid from_string(std::basic_string<ch, char_traits, allocator> const& s); uuid from_string(const char* name); uuid from_string(const wchar_t* name); OR class string_generator //not a good name { public: typedef uuid result_type; template <typename ch, typename char_traits, typename allocator> uuid operator()(std::basic_string<ch, char_traits, allocator> const& name); uuid operator()(const char* name); // use std::strlen uuid operator()(const wchar_t* name); // use std::wcslen };
// 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?
Yes.
Also, when is uuid(gen) significantly more convenient than uuid(gen())? I'd have thought that the copy constructor was plenty.
It is more convenient than the copy constructor for generators like random_generator. Eg: random_generator<> gen; uuid u(gen); instead of uuid u = gen(); or uuid u(gen()); It is _significantly_ more convenient? I guess not. Hmm, good point.
// 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.
Sure, I don't mind putting it in. I'm happy to follow shared_ptr's lead on this.
// 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.)
I removed them because of the statics. operator>> can continue to handle them. I agree that it's easy to stream them explicitly, or create a modifier to use similar to: uuid u; cout << with_braces(u);
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.
Yes, so that one can create a nil uuid explicitly.
On that note, are you including the name-based UUID name spaces anywhere?
No, I didn't. But I can.
// 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<>).
Good idea. But there is no md5 implementation in boost, and I'm not planning to write one (unless there is significant demand).
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.
Good point. Maybe just random_based_generator and version_random_based.
uuid operator()(const wchar_t* name); // use std::wcslen
What happens here if there are wchar_ts in the stream outside of 0xFF?
This function will treat the stream a sequence of bytes and will have the same effect as using the ByteInputIterator version on the stream. So each wchar_t will be treated as sizeof(wchar_t) bytes.
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?
It sounds like people just want defaults. It is also easy to create a generator using one of these as a pattern to use a different platform function/parameter.
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 convertible 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.
Good point.
What's this used for?
I am just trying to add flexibility.
Would it worth be adding one for message UUID generators too?
What do you mean by message uuid generators?
Thanks for your work on this, ~ Scott
Regards, Andy Tompkins