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

Subject: Re: [boost] [Review] UUID library (mini-)review starts today, November 23rd
From: Andy Tompkins (atompkins_at_[hidden])
Date: 2008-11-24 20:59:05

On Mon, 24 Nov 2008 16:59:10 -0000, "Paul A. Bristow"
<pbristow_at_[hidden]> said:

< snip >

> Re-reading the docs, I can't see any reasons against acceptance.
> 
> Nits:
> 
> I note that the docs uuid.html copyright date is still only 2006.

Oops, I'll fix this.

> And it does not list/link all the tests.

I will add them.

> Mis spelling of 'hexidecimal'
> The external representation of a uuid is a string of hexidecimal digits

I'll fix this.  (How do others spell check html?)

> Note: boost::uuids::uuid::size() always returnes 16.
> Mispelled 'returnes'

I'll fix this too.

> Is there a reason why create does not also take a std::string (with
> default
> length .size() as parameter?)
>     // Static functions
>     static uuid create(uuid const& namespace_uuid, char const* name, int
> name_length);
> 
> I assumed it would exist and was surprised when it didn't.
> 
> (Should the name_length have a default value? C-string size - 1?)

I don't have a preference.  The create function was done this way so
that it could take a block of memory and not just strings, but thinking
about it now, void* would be better for this.  It does not sound like
this is useful and I should just have the function take a
std::basic_string.  I've also considered changing this to a function
object similar to basic_uuid_generator instead of a static function. 
What do people want?

> I also believe that a really basic example would be useful.  This helps
> novices.
> 
> A little demo I knocked up quickly attached.  (MSVC 8 Sp1)

With your permission, I'll include your example.

> It reveals that the hated 4996 warnings are triggered (at default MS
> warning
> level 3).
> I think these need to be suppressed with push'n'popping.
> 
> Also I got a faceful of these at warning level 4. Again they should be
> suppressed.
> 1>I:\boost_1_37_0\boost/random/detail/pass_through_engine.hpp(49) :
> warning
> C4512:
> 'boost::random::detail::pass_through_engine<UniformRandomNumberGenerator>'
> :
> assignment operator could not be generated

I will suppress these warnings as in your example.

> 1>H:\uuid\boost/uuid/uuid.hpp(364) : warning C4244: '=' : conversion from
> 'int' to 'char', possible loss of data
> 
> Should be silenced using a static_cast?
> 
>         c = static_cast<ch>(is.peek());

I will fix this.

> But overall this seems 'OK to ship'.
> 
> HTH
> 
> Paul
> 
> ---
> Paul A. Bristow
> Prizet Farmhouse
> Kendal, UK   LA8 8AB
> [phone], mobile [phone]
> pbristow_at_[hidden]
> 

Thanks,
  Andy Tompkins.