[boost] [review] Boost.Guid

From: Peter Dimov (pdimov_at_[hidden])
Date: 2007-05-05 09:16:46

I think that the library is useful, needed and should be accepted subject to 
the comments below.

* The proper (standard) name for the component is UUID. GUID is a 
Microsoft-specific term for the same thing, which has later been adopted to 
mean any kind of globally unique identifier. We should stick to the standard 
name unless there are clear reasons to avoid it.

* I agree that the time(0) seed is unacceptable. A good source of entropy 
probably deserves its own library. It's also not easy to make it 
header-only. One compromise could be for the create function to take an 
Engine argument, but this takes away the simplicity.

* The dependency on Boost.Threads in its present form can be dropped since 
it doesn't make the library thread safe. There are a number of function 
static variables, many of them unnecessary.

* The string constructors should probably be optimized to not use iostream 
extraction, but this is an implementation detail.

* The guid class is almost a container, but not quite. It is not clear why 
output_bytes has been preferred over begin/end returning const iterators. 
These could come for free from the underlying array<>.

* The dependency on uint8_t is unnecessary; the underlying type should be 
unsigned char. There is no need to insist on exactly 8 bits.

* The c == '{' comparison in operator>> seems to be missing a 'widen' call. 
There is no guarantee that '{' and L'{' compare equal.

* It isn't clear why a guid is marked as a primitive type for serialization 
purposes. Not doing so would allow the macro BOOST_GUID_ENABLE_SERIALIZATION 
to be dropped since

    friend class boost::serialization::access;

    template<class Archive>
    void serialize(Archive &ar, const unsigned int /* file_version */){
        ar & data_;
    }

does not seem to need any serialization headers and is therefore zero-cost.