From: Andy (atompkins_at_[hidden])
Date: 2007-05-07 11:19:06
"Peter Dimov" <pdimov_at_[hidden]> wrote in
news:00dd01c78f17$ba7e5610$6407a80a_at_pdimov2:
> 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.
This has been suggested before and I will gladly change it to uuid.
>
> * 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.
See another post.
http://article.gmane.org/gmane.comp.lib.boost.devel/158740
I hope that the defaults will keep 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.
Agreed.
>
> * The string constructors should probably be optimized to not use
> iostream extraction, but this is an implementation detail.
I can look into this.
>
> * 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<>.
Great idea! I didn't want to make it easy to directly change values. I
didn't think of providing just const iterators.
>
> * The dependency on uint8_t is unnecessary; the underlying type should
> be unsigned char. There is no need to insist on exactly 8 bits.
Ok. If unsigned char is more than 8 bits then sizeof(guid) would be
larger. Maybe this is not an issue.
>
> * The c == '{' comparison in operator>> seems to be missing a 'widen'
> call. There is no guarantee that '{' and L'{' compare equal.
Thanks. I get a little lost with this sort of stuff sometimes. Just to
help me, who provides the 'widen' function? The stream?
>
> * 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.
I want to read the other post about this before I reply.
Andy.