Re: [Boost-users] [Review] UUID library (mini-)review starts today, November 23rd

On Wed, 3 Dec 2008 15:57:45 -0500, "Scott McMurray" said:

On Wed, Dec 3, 2008 at 15:19, muhammadchang wrote:

...

...

"behaves and cost, exactly the same as any built in type" and the "default constructor [...] generates a valid random uuid" seem rather opposed. int() is not a random int, it's 0. No fundamental type -- no standard library type either, that I can think of -- has T() != T(), yet you want uuid() != uuid().

You are right, there is an apparent contradiction in my wording above. By "behaves and cost", I mean implementing comparison operators, inserters and extractors, for example. Where applicable, of course, and with minimum overhead. But an UUID is more than its bare 128-bit representation. IMHO, what justifies the truly exceptional uuid() != uuid() is the "unique" in "universally unique id".

But it's the mapping of the ID to something that's unique, not the objects themselves. The only time you can do something useful with a UUID is when you have two that are the same, so I still don't think breaking normal semantics is justified.

I also think that uuid() == uuid() is a good thing. I do understand the other point of view and where it is coming from. It really does make me think that there should not be a default constructor. Then, one must be explicit if they want a 'null' uuid, something like: boost::uuids::uuid u1(boost::uuids::null()); and one must be explicit if they want a unique uuid, something like: boost::uuids::uuid u2(boost::uuids::uuid_generator());

The usage I was looking at for my project never needs the UUID library to generate one itself, in fact. They're all generated in the database (http://dev.mysql.com/doc/refman/5.0/en/miscellaneous-functions.html#function...), but the class is still useful for parsing and serialization.

Changing tacks somewhat...

I was just looking deeper into the implementation, and saw some things that worry me. For example, this looks like an aliasing violation in uuid.hpp line 279:

*reinterpret_cast(&data[i]) = _v_gen();

I do not believe this is an aliasing violation.

It's at least clearly byte-order-dependant, which isn't good, since I'd expect a generator seeded the same way to always produce the same UUIDs, regardless of architecture.

Hmm, I was only trying to be efficient. The generator produces values of size = sizeof(unsigned long) and I wanted to use all those bytes. Do you have a suggestion that would improve this?

I'm pleased to see proper shifting and masking in the SHA1 code, which made me think of something: Why not just make the UUID class a PoD? It has an architecture-independent in-memory format (rfc4122, section 4.1.2, which is already followed), so turning it into one would be straight- forward, and would allow it to be very easily & safely splatted to and from files.

I do really like having a class encapsulate the data and functions. Also, I don't have much experience with platforms where sizeof(int) != 4 or sizeof(char) != 1, and I want to make sure that I can do as you suggest.

This would require moving the constructors and such to generators, stream operators, and similar, but I think those are positive changes in any case. It certainly solves the "what should the constructor do" problem, and places all the various generators (nil, random, name, time, ...) at equal footing.

I also think that all the ways to create a uuid can be made as generators, and by not providing a default constructor will keep them equal.

It's a value type, so let's make it as much like an int as possible.

Regards, Andy Tompkins