Hartmut Kaiser | 24 Nov 01:10

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

Hi all,

The mini-review of Andy Tompkins UUID library starts today, November 23rd
2008, and will end on November 30th. 
I really hope to see your vote and your participation in the discussions on
the Boost mailing lists!

The library can be downloaded from the vault here (it's the file
uuid_v13.zip in the topmost directory): http://tinyurl.com/6xx28b

The initial review of the UUID library ended with a provisional acceptance
(read here: http://article.gmane.org/gmane.comp.lib.boost.user/27774/). 

This mini review is meant to allow for a final decision after looking at the
changed parts of the library. Here is a list of things fixed and changed:

- fixed the licensing issues revealed by the initial review
- fixed the security vulnerability revealed by the initial review
- renamed to uuid, moved all classes, functions, etc. to namespace
boost::uuids
- implemented sha1 hash function (thus no license problem)
- new class basic_uuid_generator to create random-based uuids.  The random
number generator is no longer hard coded.  It can use any random number
generator, default is boost::mt19937
- implemented a good seed function for random number generators
- all functions are now reentrant, all classes are as thread safe as an int
and the library is no longer dependent on Boost.Thread

---------------------------------------------------

(Continue reading)

Dave Jenkins | 24 Nov 14:58
Favicon

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


> The library can be downloaded from the vault here (it's the file
> uuid_v13.zip in the topmost directory): http://tinyurl.com/6xx28b

This link didn't work for me.  Is there a problem with it?

Dave Jenkins
Roman Perepelitsa | 24 Nov 15:06

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

2008/11/24 Dave Jenkins <david <at> jenkins.net>


The library can be downloaded from the vault here (it's the file
uuid_v13.zip in the topmost directory): http://tinyurl.com/6xx28b

This link didn't work for me.  Is there a problem with it?

Works for me. You can also try this: http://www.boostpro.com/vault/index.php?action=downloadfile&filename=uuid_v13.zip

Roman Perepelitsa.
_______________________________________________
Boost-users mailing list
Boost-users <at> lists.boost.org
http://lists.boost.org/mailman/listinfo.cgi/boost-users
Paul A. Bristow | 24 Nov 17:59

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

> -----Original Message-----
> From: boost-bounces <at> lists.boost.org [mailto:boost-bounces <at> lists.boost.org]
On
> Behalf Of Hartmut Kaiser
> Sent: 24 November 2008 00:14
> To: boost <at> lists.boost.org; boost-users <at> lists.boost.org; boost-
> announce <at> lists.boost.org
> Subject: [boost] [Review] UUID library (mini-)review starts today,
November 23rd
> 
> The mini-review of Andy Tompkins UUID library starts today, November 23rd
> 2008, and will end on November 30th.
> I really hope to see your vote and your participation in the discussions
on
> the Boost mailing lists!
> 
> The library can be downloaded from the vault here (it's the file
> uuid_v13.zip in the topmost directory): http://tinyurl.com/6xx28b
> 
> The initial review of the UUID library ended with a provisional acceptance
> (read here: http://article.gmane.org/gmane.comp.lib.boost.user/27774/).

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.

And it does not list/link all the tests.

Mis spelling of 'hexidecimal'

The external representation of a uuid is a string of hexidecimal digits

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

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 also believe that a really basic example would be useful.  This helps
novices.

A little demo I knocked up quickly attached.  (MSVC 8 Sp1)

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

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());

But overall this seems 'OK to ship'.

HTH

Paul

---
Paul A. Bristow
Prizet Farmhouse
Kendal, UK   LA8 8AB
+44 1539 561830, mobile +44 7714330204
pbristow <at> hetp.u-net.com

//  Copyright Paul A. Bristow.
//  Permission to copy, use, modify, sell and
//  distribute this software is granted provided this copyright notice appears
//  in all copies. This software is provided "as is" without express or implied
//  warranty, and with no claim as to its suitability for any purpose.

// Distributed under the Boost Software License, Version 1.0. (See
// accompanying file LICENSE_1_0.txt or copy at
// http://www.boost.org/LICENSE_1_0.txt)

//  libs/uuid/example/example.cpp  -------------------------------//

#define _CRT_SECURE_NO_WARNINGS
// h:\uuid\boost\uuid\seed_rng.hpp(132) : warning C4996: 'fopen':
// This function or variable may be unsafe. Consider using fopen_s instead.
// To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.
// c:\program files\microsoft visual studio 9.0\vc\include\stdio.h(237) :
// see declaration of 'fopen'
#define _SCL_SECURE_NO_WARNINGS 

#include <boost/uuid/uuid.hpp>
#include <boost/uuid/seed_rng.hpp>
#include <iostream>
#include <string>
#include <vector>

using std::cout;
using std::endl;
using std::string;
using std::vector;
using std::copy;
using std::ostream_iterator;

int main()
{
  using boost::uuids::uuid;
  using boost::uuids::uuid_generator;
  using boost::uuids::showbraces;
  using boost::uuids::noshowbraces;

  uuid u; // Default constructor makes a null string - boring!
  cout << u << endl; // 00000000-0000-0000-0000-000000000000
  if (u.is_null())
  {
    cout << "uuid is null!" << endl; // uuid is null!
  }
  // Create from a std string.
  string s1("01234567-89ab-cdef-0123-456789abcdef");
  uuid us1(s1);
  cout << us1 << endl; // 01234567-89ab-cdef-0123-456789abcdef
  // Or a C string.
  const char* s2 = "{76543210-89ab-cdef-0123-456789abcdef}";
  uuid uc1(s2);
  cout << uc1 << endl; // 76543210-89ab-cdef-0123-456789abcdef
  // and also show {} braces around string.
  cout << showbraces << uc1 << endl; // {76543210-89ab-cdef-0123-456789abcdef}
  cout << uc1 << endl; // {76543210-89ab-cdef-0123-456789abcdef}

  // Get at the UUID individual bytes.
  vector<char> v(u.size()); // Always 16.
  copy(us1.begin(), us1.end(), v.begin());
  cout << "Length in bytes is " << v.size() << ", and values are: " << endl;
  for (int i = 0; i < 16; i++)
  {
    cout << int(v[i]) << " ";
  } // 1 35 69 103 -119 -85 -51 -17 1 35 69 103 -119 -85 -51 -17
  cout << endl; 

  // Create uuid from name.
  uuid dns_namespace_uuid("6ba7b810-9dad-11d1-80b4-00c04fd430c8");
  const char* url = "www.boost.org";
  uuid uurl = boost::uuids::uuid::create(dns_namespace_uuid, url, 13);
  cout << uurl << endl;
  string suurl = uurl.to_string(); 
  cout << suurl << endl; // ccb046db-74eb-53c9-9b6b-3ce5740dd29e

  // Generate uuid from random number, different every time.
  uuid_generator ur; //
  cout << showbraces << ur() << endl; // {f850857f-86cb-4518-ba45-c826aac45a03}
  cout << ur() << endl; // {b04004aa-c261-4a91-a79f-357d71780d0c}
  cout << ur() << endl; // {b305b7e1-80ef-4fa9-beed-c26b86a08cbb}
  // Note showbraces 'sticks' until noshowbraces.
  cout << noshowbraces << ur() << endl; // b305b7e1-80ef-4fa9-beed-c26b86a08cbb
  uuid u1 = ur();
  uuid u2 = ur();
  if (u1 == u2)
  {
    cout << "Ooops - UUIDs should be unique!" << endl;
  }

} // int main()

/*

Output:

1>Autorun "h:\uuid\libs\uuid\Example\Example\Debug\Example.exe"
1>00000000-0000-0000-0000-000000000000
1>uuid is null!
1>01234567-89ab-cdef-0123-456789abcdef
1>76543210-89ab-cdef-0123-456789abcdef
1>{76543210-89ab-cdef-0123-456789abcdef}
1>{76543210-89ab-cdef-0123-456789abcdef}
1>Length in bytes is 16, and values are: 
1>1 35 69 103 -119 -85 -51 -17 1 35 69 103 -119 -85 -51 -17 
1>{ccb046db-74eb-53c9-9b6b-3ce5740dd29e}
1>ccb046db-74eb-53c9-9b6b-3ce5740dd29e
1>{d0b211fd-6e42-4112-8270-dd470f0ccb08}
1>{6fa9bc27-ee4b-4d52-ac83-0868e94698ab}
1>{9fc020e5-54ea-4e42-b716-7b8c89e5b5f0}
1>b3e3936b-e6f1-4909-915a-8c7a4aba76a6
1>Build Time 0:02

*/
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Andy Tompkins | 25 Nov 02:59
Favicon

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

On Mon, 24 Nov 2008 16:59:10 -0000, "Paul A. Bristow"
<pbristow <at> hetp.u-net.com> 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
> +44 1539 561830, mobile +44 7714330204
> pbristow <at> hetp.u-net.com
> 

Thanks,
  Andy Tompkins. 
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

Paul A. Bristow | 25 Nov 10:36

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

> -----Original Message-----
> From: boost-bounces <at> lists.boost.org [mailto:boost-bounces <at> lists.boost.org]
On
> Behalf Of Andy Tompkins
> Sent: 25 November 2008 01:59
> To: boost_dev
> Subject: Re: [boost] [Review] UUID library (mini-)review starts today,
November 23rd

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

Well if you write in Quickbook, you can use your regular spell checker - I
use Textpad for example.
(It also produces hyperlinked pdfs - and should include automatic indexing
soon).

But of course it produces lots of false alarms :-(

(But don't redo the docs just for this).

> > 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?

Are these mutually exclusive?  I don't have a strong view - I'm just
reported what I assumed.
(But then assumption is the mother of all foul-ups ;-)

> > I also believe that a really basic example would be useful.  This helps
> > novices.

> With your permission, I'll include your example.

Of course - see licence ;-)

You should (be able to) remove the #defines if you sort out the details
below.

And other usages would be useful - For example, creating unique filenames...
Examples are often more useful than manuals.

> I will suppress these warnings as in your example.
Good.

Paul

---
Paul A. Bristow
Prizet Farmhouse
Kendal, UK   LA8 8AB
+44 1539 561830, mobile +44 7714330204
pbristow <at> hetp.u-net.com

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

Andy Tompkins | 26 Nov 04:39
Favicon

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


On Tue, 25 Nov 2008 09:36:44 -0000, "Paul A. Bristow" <pbristow <at> hetp.u-
net.com> said:
> > -----Original Message----- From: boost-bounces <at> lists.boost.org [mailto:boost-
> > bounces <at> lists.boost.org]
> On
> > Behalf Of Andy Tompkins Sent: 25 November 2008 01:59 To: boost_dev
> > Subject: Re: [boost] [Review] UUID library (mini-)review starts
> > today,
> November 23rd
>
> > I'll fix this.  (How do others spell check html?)
>
> Well if you write in Quickbook, you can use your regular spell checker
> - I use Textpad for example. (It also produces hyperlinked pdfs - and
> should include automatic indexing soon).
>
> But of course it produces lots of false alarms :-(
>
> (But don't redo the docs just for this).

I'm hoping to have someone else convert the docs for me if the uuid
library is accepted.

> > > 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?
>
> Are these mutually exclusive?  I don't have a strong view - I'm just
> reported what I assumed. (But then assumption is the mother of all
> foul-ups ;-)

No, they are not mutually exclusive.  To me, having both functions is a
stronger reason to change the interface from static functions to a
function object.

>
> > > I also believe that a really basic example would be useful.  This
> > > helps novices.
>
> > With your permission, I'll include your example.
>
> Of course - see licence ;-)

Thanks!

> You should (be able to) remove the #defines if you sort out the
> details below.
>
> And other usages would be useful - For example, creating unique
> filenames... Examples are often more useful than manuals.

Good idea.  I will put this example in and others as they come up.

> > I will suppress these warnings as in your example.
> Good.
>
> Paul
>
>
> ---
> Paul A. Bristow Prizet Farmhouse Kendal, UK   LA8 8AB +44 1539 561830,
> mobile +44 7714330204 pbristow <at> hetp.u-net.com
>

Thanks,
  Andy Tompkins
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

Christian Henning | 24 Nov 18:45

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

Hi there,

>
> Please always state in your review, whether you think the library should be
> accepted as a Boost library!

I vote for acceptance.

>
> Additionally please consider giving feedback on the following general
> topics:
>
> - What is your evaluation of the design?

The design I like and fits into my world of c++ programming.

> - What is your evaluation of the implementation?

Looks good. I like the programming style. I would recommend to clean
up the files, in particular the commented out headers. Also, the
file uuid_serialize.hpp contains only commented out code besides the
macro instantiation. Is that functionality working?

How about having a uuid_all.hpp header?

> - What is your evaluation of the documentation?

Good.

> - What is your evaluation of the potential usefulness of the library?

Huge. We are using UUID's a lot and this library seems to be a good
candidate for the next code refactoring.

> - Did you try to use the library?  With what compiler?  Did you have any
> problems?

Yes, I compiled the test program which Paul Bristow posted and it
worked without a glitch. I'm using Visual Studio 8.

> - How much effort did you put into your evaluation? A glance? A quick
> reading? In-depth study?

A quick reading.

> - Are you knowledgeable about the problem domain?

I'm a user.

Thanks for another very useful lib.

Christian
Andy Tompkins | 25 Nov 02:23
Favicon

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

On Mon, 24 Nov 2008 12:45:41 -0500, "Christian Henning"
<chhenning <at> gmail.com> said:

< snip >

> > - What is your evaluation of the implementation?
> 
> Looks good. I like the programming style. I would recommend to clean
> up the files, in particular the commented out headers. Also, the
> file uuid_serialize.hpp contains only commented out code besides the
> macro instantiation. Is that functionality working?
> 
> How about having a uuid_all.hpp header?

If users want this, I have no problem creating a uuid_all.hpp header.

I'm also considering creating a uuid_fwd.hpp header.

< snip >

> 
> Christian
> 

Thanks,
  Andy Tompkins
Tim Blechmann | 24 Nov 18:56
Favicon

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

> The mini-review of Andy Tompkins UUID library starts today, November
> 23rd 2008, and will end on November 30th.

i just did a quick compile/testsuite test for now,  and i see three 
trivial issues:

1. it doesn't compile with gcc-4.4, because of a missing include.
patch:
--- a/boost/uuid/seed_rng.hpp
+++ b/boost/uuid/seed_rng.hpp
@@ -26,6 +26,7 @@
 #include <memory.h>^M
 #include <ctime>^M
 #include <cstdlib>^M
+#include <cstdio>^M
 #include <boost/uuid/sha1.hpp>^M
 //#include <boost/nondet_random.hpp> //forward declare boost::random_device^M
 ^M

2. the testsuite fails on x86_64 because it is using hardcoded 32-bit 
hash values:
Running 1 test case...
libs/uuid/test/test_uuid.cpp(146): error in "test_main_caller( argc, argv )": check
uuid_hasher(uuid()) == 3565488559U failed [17562157925649023279 != 3565488559]
libs/uuid/test/test_uuid.cpp(147): error in "test_main_caller( argc, argv )": check
uuid_hasher(u1) == 4159045843U failed [6622376135548557651 != 4159045843]
libs/uuid/test/test_uuid.cpp(148): error in "test_main_caller( argc, argv )": check
uuid_hasher(u2) == 2713274306U failed [4700797755868797762 != 2713274306]

*** 3 failures detected in test suite "Test Program"

3. the sources use cr+lf eol style, i guess correctly importing the
sources into subversion will fix this

maybe i find some time to do a full review later this week. anyway, 
i have been using the library for quite some time and would like to 
see it being part of boost. thus i vote for acceptance.

cheers, tim

--

-- 
tim <at> klingt.org
http://tim.klingt.org

The composer makes plans, music laughs.
  Morton Feldman
Andy Tompkins | 25 Nov 02:34
Favicon

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

On Mon, 24 Nov 2008 17:56:20 +0000 (UTC), "Tim Blechmann"
<tim <at> klingt.org> said:
> > The mini-review of Andy Tompkins UUID library starts today, November
> > 23rd 2008, and will end on November 30th.
> 
> i just did a quick compile/testsuite test for now,  and i see three 
> trivial issues:
> 
> 1. it doesn't compile with gcc-4.4, because of a missing include.
> patch:
> --- a/boost/uuid/seed_rng.hpp
> +++ b/boost/uuid/seed_rng.hpp
> @@ -26,6 +26,7 @@
>  #include <memory.h>^M
>  #include <ctime>^M
>  #include <cstdlib>^M
> +#include <cstdio>^M
>  #include <boost/uuid/sha1.hpp>^M
>  //#include <boost/nondet_random.hpp> //forward declare
>  boost::random_device^M
>  ^M

I'll fix this.

> 
> 2. the testsuite fails on x86_64 because it is using hardcoded 32-bit 
> hash values:
> Running 1 test case...
> libs/uuid/test/test_uuid.cpp(146): error in "test_main_caller( argc, argv
> )": check uuid_hasher(uuid()) == 3565488559U failed [17562157925649023279
> != 3565488559]
> libs/uuid/test/test_uuid.cpp(147): error in "test_main_caller( argc, argv
> )": check uuid_hasher(u1) == 4159045843U failed [6622376135548557651 !=
> 4159045843]
> libs/uuid/test/test_uuid.cpp(148): error in "test_main_caller( argc, argv
> )": check uuid_hasher(u2) == 2713274306U failed [4700797755868797762 !=
> 2713274306]
> 
> *** 3 failures detected in test suite "Test Program"

Hmm, I assume that the value that uuid_hasher produces is correct since
I
mimicked the way Boost.Hash does it.  So, I believe I just need to
adjust the
code in test_uuid.cpp in a 64 bit environment where sizeof(std::size_t)
!= 32?

> 
> 
> 3. the sources use cr+lf eol style, i guess correctly importing the
> sources into subversion will fix this

I believe so this to be true.

> 
> 
> maybe i find some time to do a full review later this week. anyway, 
> i have been using the library for quite some time and would like to 
> see it being part of boost. thus i vote for acceptance.
> 
> cheers, tim
> 
> -- 
> tim <at> klingt.org
> http://tim.klingt.org
> 
> The composer makes plans, music laughs.
>   Morton Feldman
> 

Thanks,
  Andy Tompkins
Tim Blechmann | 25 Nov 10:37
Favicon

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

>> libs/uuid/test/test_uuid.cpp(148): error in "test_main_caller( argc,
>> argv )": check uuid_hasher(u2) == 2713274306U failed
>> [4700797755868797762 != 2713274306]
>> 
>> *** 3 failures detected in test suite "Test Program"
> 
> Hmm, I assume that the value that uuid_hasher produces is correct since
> I
> mimicked the way Boost.Hash does it.  So, I believe I just need to
> adjust the
> code in test_uuid.cpp in a 64 bit environment where sizeof(std::size_t)
> != 32?

right ... i suppose the hardcoded value in test_uuid.cpp is only correct on
32 bit platforms ...

cheers, tim

--

-- 
tim <at> klingt.org
http://tim.klingt.org

The price an artist pays for doing what he wants is that he has to do
it.
  William S. Burroughs
Rutger ter Borg | 25 Nov 20:03

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

Hartmut Kaiser wrote:
> - What is your evaluation of the design?
> - What is your evaluation of the implementation?
> - What is your evaluation of the documentation?
> - What is your evaluation of the potential usefulness of the library?
> - Did you try to use the library?  With what compiler?  Did you have any
> problems?
> - How much effort did you put into your evaluation? A glance? A quick
> reading? In-depth study?
> - Are you knowledgeable about the problem domain?
> 
> Regards Hartmut
> Review Manager

Herewith my non-exhaustive review:

I've looked at the library, good work, it looks very interesting. I did a
quick reading, and in particular, I paid attention to the many of its
intended uses (in the Rationale), and how this would have to be done in
practice (the Interface).

 * The design notes state that it is based on X.667-E. This document
describes three construction mechanisms: time-based, rng-based, and
string-based. The proposal doesn't have the time-based constructor. Is
there a reason why this is missing?
 * Is the boost namespace that cluttered that it isn't possible to
hold "boost::uuid" in it? I think boost::uuids::uuid is kind of repetitive.
(Or is there a policy against putting stuff in the boost root-namespace?)
 * Construction is described in "Constructors" and "Construction" -- this
could perhaps lead to confusion?
 * In Representation: Is there a reason for a .to_string() member function
instead of an operator std::string()? I.e., it reduces std::string s =
u.to_string() to std::string s( u );
 * Considering the examples in the Rationale and the available constructors:
I'm curious how to easily "tag an object" using the provided constructors.
By a random number? Or could I just pass the address of the object and use
a string-based method? In other words, it would be nice if such an example
from the Rationale would be topic of an example.

I vote for acceptance into boost, given that just a bit more attention is
paid to a potential user's convenience. 

Kind regards,

Rutger

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

Andy Tompkins | 26 Nov 04:56
Favicon

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

On Tue, 25 Nov 2008 20:03:22 +0100, "Rutger ter Borg"
<rutger <at> terborg.net> said:

< snip >

> Herewith my non-exhaustive review:
>
> I've looked at the library, good work, it looks very interesting. I
> did a quick reading, and in particular, I paid attention to the many
> of its intended uses (in the Rationale), and how this would have to be
> done in practice (the Interface).
>
>  * The design notes state that it is based on X.667-E. This document
>    describes three construction mechanisms: time-based, rng-based, and
>    string-based. The proposal doesn't have the time-based constructor.
>    Is there a reason why this is missing?

Only because I'm not sure how to implement the time-based construction
mechanism a portable way.

>  * Is the boost namespace that cluttered that it isn't possible to
>    hold "boost::uuid" in it? I think boost::uuids::uuid is kind of
>    repetitive. (Or is there a policy against putting stuff in the
>    boost root-namespace?)

I did this because of
http://www.boost.org/development/requirements.html#Naming_consistency
But I have no problem doing something else if this list wants me too.

>  * Construction is described in "Constructors" and "Construction" --
>    this could perhaps lead to confusion?

Do you mean "Constructors" and "Creation"?  Hmm, I'll give this
some thought.

>  * In Representation: Is there a reason for a .to_string() member
>    function instead of an operator std::string()? I.e., it reduces
>    std::string s =
> u.to_string() to std::string s( u );

I tend to avoid implicit conversions.  One can also use
boost::lexical_cast<std::string>(uuid).  Again, if this list wants
operator std::string() and the like, I will add it.

>  * Considering the examples in the Rationale and the available
>    constructors: I'm curious how to easily "tag an object" using the
>    provided constructors. By a random number? Or could I just pass the
>    address of the object and use a string-based method? In other
>    words, it would be nice if such an example from the Rationale would
>    be topic of an example.

I think an example for this is a good idea.  On way would be:

class Foo {
public:
  Foo()
    // create a temporary generator (or pass one in the constructor
    // or create a singleton) and initialize m_uuid with it.
    : m_uuid(boost::uuids::uuid_generator()())
    {}

private:
  const boost::uuids::uuid m_uuid;
};

>
> I vote for acceptance into boost, given that just a bit more attention
> is paid to a potential user's convenience.
>
> Kind regards,
>
> Rutger

Thanks,
  Andy.
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

Michael Marcin | 26 Nov 18:14

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

Andy Tompkins wrote:
> 
> Only because I'm not sure how to implement the time-based construction
> mechanism a portable way.
> 

I'm not sure what is required but perhaps the newly sandboxed Chrono 
library can help here?

> 
> I tend to avoid implicit conversions.  One can also use
> boost::lexical_cast<std::string>(uuid).  Again, if this list wants
> operator std::string() and the like, I will add it.
> 

Please no.

--

-- 
Michael Marcin

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

Andy Tompkins | 27 Nov 05:56
Favicon

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

On Wed, 26 Nov 2008 11:14:38 -0600, "Michael Marcin"
<mike.marcin <at> gmail.com> said:
> Andy Tompkins wrote:
> >
> > Only because I'm not sure how to implement the time-based
> > construction mechanism a portable way.
> >
>
> I'm not sure what is required but perhaps the newly sandboxed Chrono
> library can help here?

I will take a look at Chrono, it may help.  The time-based construction
ideally requires a MAC address, and I don't know how to get it portably.

> >
> > I tend to avoid implicit conversions.  One can also use
> > boost::lexical_cast<std::string>(uuid).  Again, if this list wants
> > operator std::string() and the like, I will add it.
> >
>
> Please no.

This is the second no vote, plus mine makes 3.  I won't add an implicit
conversion to std::string.

> --
> Michael Marcin
>

Regards,
  Andy Tompkins
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

Hartmut Kaiser | 27 Nov 13:50

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

> > > Only because I'm not sure how to implement the time-based
> > > construction mechanism a portable way.
> > >
> >
> > I'm not sure what is required but perhaps the newly sandboxed Chrono
> > library can help here?
> 
> I will take a look at Chrono, it may help.  The time-based construction
> ideally requires a MAC address, and I don't know how to get it
> portably.

Why not leaving the burden of retrieving the MAC address to the user, then?

Regards Hartmut

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

Michael Marcin | 1 Dec 23:08

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

Hartmut Kaiser wrote:
>>>> Only because I'm not sure how to implement the time-based
>>>> construction mechanism a portable way.
>>>>
>>> I'm not sure what is required but perhaps the newly sandboxed Chrono
>>> library can help here?
>> I will take a look at Chrono, it may help.  The time-based construction
>> ideally requires a MAC address, and I don't know how to get it
>> portably.
> 
> Why not leaving the burden of retrieving the MAC address to the user, then?
> 

Because portably retrieve a good MAC address is hard and I don't want to 
have to implement that logic.

My laziness aside it probably is reasonable to say this is outside of 
the scope this library. Perhaps another utility library to obtain the 
mac address can be authored by one of the people requesting this 
feature? They are probably going to have to implement that code to use 
it anyways.

--

-- 
Michael Marcin

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

Stefano Delli Ponti | 27 Nov 14:25

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

Hartmut Kaiser wrote:
>>>> Only because I'm not sure how to implement the time-based
>>>> construction mechanism a portable way.
>>>>
>>> I'm not sure what is required but perhaps the newly sandboxed Chrono
>>> library can help here?
>> I will take a look at Chrono, it may help.  The time-based construction
>> ideally requires a MAC address, and I don't know how to get it
>> portably.
> 
> Why not leaving the burden of retrieving the MAC address to the user, then?
> 

That was what I was going to suggest too.

> Regards Hartmut
> 
> 
> _______________________________________________
> Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
> 

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

Andy Tompkins | 28 Nov 02:44
Favicon

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

On Thu, 27 Nov 2008 06:50:37 -0600, "Hartmut Kaiser"
<hartmut.kaiser <at> gmail.com> said:
> > > > Only because I'm not sure how to implement the time-based
> > > > construction mechanism a portable way.
> > >
> > > I'm not sure what is required but perhaps the newly sandboxed
> > > Chrono library can help here?
> >
> > I will take a look at Chrono, it may help.  The time-based
> > construction ideally requires a MAC address, and I don't know how to
> > get it portably.
>
> Why not leaving the burden of retrieving the MAC address to the
> user, then?

That is a _really_ good idea!  I will spend some time and take a serious
look at implementing a time-based uuid generator.

>
> Regards Hartmut

Thanks,
  Andy Tompkins
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

Rutger ter Borg | 27 Nov 18:03

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

Andy Tompkins wrote:
>
> I will take a look at Chrono, it may help.  
>

The ptime class in the Boost.Date_Time library also provides different
portable constructors based on the system's time.

Kind regards,

Rutger

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

Andy Tompkins | 28 Nov 03:29
Favicon

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

On Thu, 27 Nov 2008 18:03:45 +0100, "Rutger ter Borg"
<rutger <at> terborg.net> said:
> Andy Tompkins wrote:
> >
> > I will take a look at Chrono, it may help.
> >
>
> The ptime class in the Boost.Date_Time library also provides different
> portable constructors based on the system's time.

It looks like Boost.DateTime may be a better fit and it is already in
Boost.  I need to get the current time in UTC (hopefully quite
accurately).

>
> Kind regards,
>
> Rutger

Thanks,
  Andy Tompkins
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

Manuel Fiorelli | 28 Nov 03:31
Gravatar

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

>
> Why not leaving the burden of retrieving the MAC address to the user,
> then?
>
In my opinion one can provide a "generic" generator, which requires a MAC
address; but there should be a set of platform specific generators, which
use system calls to get the hardware address.

I am not a network expert, but it seems that on Linux you can obtain the MAC
address through IOCTL

http://www.geekpage.jp/en/programming/linux-network/get-macaddr.php

even if this approach couldn't be very reliable

http://unix.derkeiler.com/Newsgroups/comp.unix.solaris/2007-06/msg00854.html

In any case one could look at the ifconfig source

http://www.koders.com/c/fidF4E339CD5847BBECCA5405DD6592D79452A56D3E.aspx?s=link

(I am not sure it is the original code :-)

For Windows user I found the article

http://www.codeguru.com/cpp/i-n/network/networkinformation/article.php/c5451

with a warning

http://www.codeguru.com/cpp/i-n/network/networkinformation/comments.php/c5451/?thread=5453

In any cases, one should add the error checking :-)

Manuel Fiorelli
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

Francois Barel | 28 Nov 07:34

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

Manuel Fiorelli wrote:
>>
>> Why not leaving the burden of retrieving the MAC address to the user,
>> then?
>>
>
> For Windows user I found the article
>
> http://www.codeguru.com/cpp/i-n/network/networkinformation/article.php/c5451
>
> with a warning
>
> http://www.codeguru.com/cpp/i-n/network/networkinformation/comments.php/c5451/?thread=5453
>

I can add another one (although I'm not sure how the OS behaves in
that case): if you actually want something unique, you cannot blindly
take the first MAC you get, you need to be smarter (and unfortunately
I don't know how to do that).

I recently saw the case where a developer thought he was getting
something unique by retrieving the first MAC address of the machine...
only to discover that on machines with VMWare installed, the first two
returned MAC addresses are actually the MAC addresses of VMWare's
virtual network adapters, which are (by default) the same everywhere.
IOW, his code was returning the same MAC address on every PC with
VMWare installed.

François
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

Manuel Fiorelli | 28 Nov 17:40
Gravatar

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

Francois Barel wrote:
> I recently saw the case where a developer thought he was getting
> something unique by retrieving the first MAC address of the machine...
> only to discover that on machines with VMWare installed, the first two
> returned MAC addresses are actually the MAC addresses of VMWare's
> virtual network adapters, which are (by default) the same everywhere.
> IOW, his code was returning the same MAC address on every PC with
> VMWare installed.
>
> François

I wasn't aware of that issue. Things are yet complicated, but your
point complicates them much more.

Manuel Fiorelli
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

Andy Tompkins | 29 Nov 06:16
Favicon

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

On Fri, 28 Nov 2008 17:40:44 +0100, "Manuel Fiorelli"
<manuel.fiorelli <at> gmail.com> said:
> Francois Barel wrote:
> > I recently saw the case where a developer thought he was getting
> > something unique by retrieving the first MAC address of the
> > machine... only to discover that on machines with VMWare installed,
> > the first two returned MAC addresses are actually the MAC addresses
> > of VMWare's virtual network adapters, which are (by default) the
> > same everywhere. IOW, his code was returning the same MAC address on
> > every PC with VMWare installed.
> >
> > François
>
> I wasn't aware of that issue. Things are yet complicated, but your
> point complicates them much more.

Wow complicated indeed.  I agree that one could make a generic uuid
generator that is given that MAC address.  And then create some platform
specific generators that work on top of the generic one.  But the
generic uuid generator will still have to be in the public interface for
those complicated cases.

> Manuel Fiorelli
>

Thanks,
Andy Tompkins
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

Manuel Fiorelli | 29 Nov 21:38
Gravatar

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

Having seen all these issues, I consider that the platform specific
generator could be deferred until one discovers how to construct them
correctly.

Isn't there some one, who knows how to get the MAC address?

Manuel Fiorelli
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

Paul A. Bristow | 1 Dec 12:44

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

> -----Original Message-----
> From: boost-bounces <at> lists.boost.org [mailto:boost-bounces <at> lists.boost.org]
On
> Behalf Of Manuel Fiorelli
> Sent: 29 November 2008 20:38
> To: boost <at> lists.boost.org
> Subject: Re: [boost] [Review] UUID library (mini-)review starts today,
November 23rd
> 
> Having seen all these issues, I consider that the platform specific
> generator could be deferred until one discovers how to construct them
> correctly.
> 
> Isn't there some one, who knows how to get the MAC address?

There is plenty of info, it sounds as though getting a MAC address may be
iffy, and not portable.

So would it not still be useful to provide an implementation that only goes
as far as expecting a MAC address as a parameter?

Leaving the user to provide the MAC address.

That doesn't preclude later adding an overloaded function which does it
'automatically' - if that is ever possible/desirable.

Paul

---
Paul A. Bristow
Prizet Farmhouse
Kendal, UK   LA8 8AB
+44 1539 561830, mobile +44 7714330204
pbristow <at> hetp.u-net.com

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

Vladimir Batov | 28 Nov 14:11

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

Andy,

Thank you for taking the criticism the right way and my apologies if some of 
my comments sound(ed) somewhat harsh. It is never my intention. I won't be 
addressing the points below directly as some issues I raised are petty (and 
I somewhat wish I did not raise them). I admit I felt uneasy with the 
proposal but could not pin-point what it was. I'll try again.

Background: We are in our project currently using uuid facility heavily and 
very dependent on it. The version we use (from memory) is something like 
(it's not necessarily exactly what we use and I'll omit some scaffolding and 
stick with the basics):

struct uuid : public std::string
{
    typedef std::string base;
    uuid() : base(generate()) {}
    explicit uuid(string const& s) : base(s) {}
    static uuid null() { return uuid(std::string()); }
    private: std::string generate();
};

On Linux the generate() function calls uuid_generate(). No one (in our 
project anyway) can be bothered choosing between uuid_generate_random() or 
uuid_generate_time() -- uuid_generate() takes care of the guess work and 
picks the best available. The same on Windows -- it'll probably be 
UuidCreate()&UuidToString(). Given that uuid above is merely a wrapper over 
OS facilities, we get known behavior, we are guaranteed "support" and an 
automatic "upgrade" if/when the standard changes or a new better algorithm 
becomes available.

For me (a collective nebulous "me") to switch to your boost::uuid it has to 
provide something to win me over. Let's see what that might be.

Does boost::uuid provide superior implementation? Maybe. If it is, that was 
not spelt out sufficiently visibly for me to pick up in the documentation. 
Therefore, as it is now the answer regretfully has to be 'no'.

Does boost::uuid provide superior interface? That issue might get very 
subjective and personal. Therefore, I'll re-focus on if boost::uuid is 
user-friendlier. Maybe but I doubt it as an ordinary user does not want to 
be bothered choosing between algorithms. If uuid_generate() and UuidCreate() 
can make that choice for me, it is good enough for the overwhelming majority 
of uuid applications. Therefore, the answer seems again 'no'.

Does boost::uuid provide superior support and a upgrade path? With all due 
respect I have to say 'no' again as your support (no matter how enthusiastic 
it is) cannot compete (on industrial/commercial scale) with vendor-backed 
support. uuid_generate(), UuidCreate() as printf() come with the system. One 
has to have really serious reasons to use something else instead.

Does boost::uuid provide superior documentation? Well, my impression is that 
it is 'no' again.

What boost::uuid does immediately entice an ordinary user with then it is 
Boost reputation. However, in all honesty it's not boost::uuid property per 
se but the reputation for quality of the Boost libraries built over those 
many years. In this thread I hear some people say "it's OK to ship". I 
disagree. Every new library has Boost reputation to uphold. It takes time to 
build such a reputation. It does not take long to lose it -- a couple of 
just "OK to ship" libraries will probably do.

I'd probably be more comfortable if boost::uuid took the approach of 
boost::thread of providing consistent interface across many platforms via 
re-use of the functionality provided by the OS (when available) rather than 
duplicating it. Then though I'd expect the interface to be given 
considerably more thought.

As for the interface I'll express my view to one of your comments below.

> I would not want the default constructor to generate a uuid. I don't
> want users to pay for that if they don't want it. Some use cases will
> not require generating uuids, only using them.

I have to disagree. I believe that the overwhelming majority of cases falls 
into the following category -- "I introduce something when/because I need 
that something". That is, by

Foo foo;
std::string str1;
std::string str2(params);
uuid id1;
uuid id2(param);

I create valid instances. Say, 'str1' is empty but it is still a valid 
string. I create 'id1' because I need an uuid. You on the other hand use

uuid id2(param); // valid instance
uuid id1; // invalid instance

where #1 creates a valid instance but #2 creates an invalid instance (it 
lacks its identifying property -- it's NOT unique). Therefore, you use the 
same (visually and behaviorally) creation mechanism to achieve *different* 
results. From the user perspective it is inconsistent and likely unexpected.

The case you are catering for with uuid() is not mainstream but quite the 
opposite -- the user introduces something that he does *not* want. Such case 
should be handled explicitly with the user actually spelling out what he 
means like

Foo::Foo()
:
    uuid_(boost::uuid::null()) // initially invalid
{}

Best,
Vladimir. 

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

Andy Tompkins | 1 Dec 06:02
Favicon

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

On Sat, 29 Nov 2008 00:14:47 +1100, "Vladimir Batov"
<batov <at> people.net.au> said:
> Andy,
>
> Thank you for taking the criticism the right way and my apologies if
> some of my comments sound(ed) somewhat harsh. It is never my
> intention. I won't be addressing the points below directly as some
> issues I raised are petty (and I somewhat wish I did not raise them).
> I admit I felt uneasy with the proposal but could not pin-point what
> it was. I'll try again.

No offense taken.  I value the criticism.  It gave my a lot to think 
about.

> Background: We are in our project currently using uuid facility
> heavily and very dependent on it. The version we use (from memory) is
> something like (it's not necessarily exactly what we use and I'll omit
> some scaffolding and stick with the basics):
>
> struct uuid : public std::string
> {
>     typedef std::string base;
>     uuid() : base(generate()) {}
>     explicit uuid(string const& s) : base(s) {}
>     static uuid null() { return uuid(std::string()); }
>   private:
>     std::string generate();
> };
>
> On Linux the generate() function calls uuid_generate(). No one (in our
> project anyway) can be bothered choosing between
> uuid_generate_random() or uuid_generate_time() -- uuid_generate()
> takes care of the guess work and picks the best available. The same on
> Windows -- it'll probably be UuidCreate()&UuidToString(). Given that
> uuid above is merely a wrapper over OS facilities, we get known
> behavior, we are guaranteed "support" and an automatic "upgrade"
> if/when the standard changes or a new better algorithm becomes
> available.
>
> For me (a collective nebulous "me") to switch to your boost::uuid it
> has to provide something to win me over. Let's see what that might be.
>
> Does boost::uuid provide superior implementation? Maybe. If it is,
> that was not spelt out sufficiently visibly for me to pick up in the
> documentation. Therefore, as it is now the answer regretfully has to
> be 'no'.

The first thing that comes to mind is size.  A uuid class that holds the 
data at text (as I believe the above does) will need 32 characters, or
32 
bytes.  Whereas boost::uuid needs the minimum of 16 bytes.  Thus twice 
as much space is required.  Also many functions (compare, assign, ...) 
likely take twice as long with the above implementation.

As a much more minor note, storing a string representation of a uuid
opens 
the door to the possibility of the string not actually representing a
valid 
uuid.  ie "1234567890abcdef!@#$%^&*().{}[]-"

> Does boost::uuid provide superior interface? That issue might get very
> subjective and personal. Therefore, I'll re-focus on if boost::uuid is
> user-friendlier. Maybe but I doubt it as an ordinary user does not
> want to be bothered choosing between algorithms. If uuid_generate()
> and UuidCreate() can make that choice for me, it is good enough for
> the overwhelming majority of uuid applications. Therefore, the answer
> seems again 'no'.

I agree that many users do not care about the algorithm used.  If they 
really don't care, is it an issue to use the
boost::uuids::uuid_generator 
since they don't really care what it does anyway?  I believe they would 
just use this function and assume all is well.  Those who care will look 
at what boost::uuids::uuid_generator does and decide for themselves.  
They will likely be happy as long as there are the options that they
want.

I do want boost::uuid to provide algorithms to create uuids.  I don't 
want platforms that don't provide an algorithm to not be able to easily 
create a uuid.  Thus I don't want boost::uuid to be just a wrapper
around 
OS functionality.

> Does boost::uuid provide superior support and a upgrade path? With all
> due respect I have to say 'no' again as your support (no matter how
> enthusiastic it is) cannot compete (on industrial/commercial scale)
> with vendor-backed support. uuid_generate(), UuidCreate() as printf()
> come with the system. One has to have really serious reasons to use
> something else instead.

I would agree, that boost::uuid will never have the support of 
industrial/commercial scale.  I now think that boost::uuid should
provide 
a windows_uuid_generator (that uses UuidCreate), a linux_uuid_generator 
(that uses uuid_generate), plus possibility others platform specific
ones, 
plus some of it's own.  Then a boost::uuids::uuid_generator could use 
one of these if available and if not, use it's own generator.  This
would 
give users the support and upgrade paths that you are talking about.

> Does boost::uuid provide superior documentation? Well, my impression
> is that it is 'no' again.

Again, I plan on improving the documentation.

> What boost::uuid does immediately entice an ordinary user with then it
> is Boost reputation. However, in all honesty it's not boost::uuid
> property per se but the reputation for quality of the Boost libraries
> built over those many years. In this thread I hear some people say
> "it's OK to ship". I disagree. Every new library has Boost reputation
> to uphold. It takes time to build such a reputation. It does not take
> long to lose it -- a couple of just "OK to ship" libraries will
> probably do.

I do hope that Boost's reputation is not all that is going for
boost::uuid.  
I value the input of this list (including yours) to help make
boost::uuid 
a library that would live up to Boost's reputation.

> I'd probably be more comfortable if boost::uuid took the approach of
> boost::thread of providing consistent interface across many platforms
> via re-use of the functionality provided by the OS (when available)
> rather than duplicating it. Then though I'd expect the interface to be
> given considerably more thought.

As above, I agree that boost::uuid should be able to use functionality 
provided by the OS.  But it should not require OS functionality to be 
useful.

> As for the interface I'll express my view to one of your
> comments below.
>
> > I would not want the default constructor to generate a uuid. I don't
> > want users to pay for that if they don't want it. Some use cases
> > will not require generating uuids, only using them.
>
> I have to disagree. I believe that the overwhelming majority of cases
> falls into the following category -- "I introduce something
> when/because I need that something". That is, by
>
> Foo foo;
> std::string str1;
> std::string str2(params);
> uuid id1;
> uuid id2(param);
>
> I create valid instances. Say, 'str1' is empty but it is still a
> valid string. I create 'id1' because I need an uuid. You on the other
> hand use
>
> uuid id2(param); // valid instance
> uuid id1; // invalid instance
>
> where #1 creates a valid instance but #2 creates an invalid instance
> (it lacks its identifying property -- it's NOT unique). Therefore, you
> use the same (visually and behaviorally) creation mechanism to achieve
> *different* results. From the user perspective it is inconsistent and
> likely unexpected.
>
> The case you are catering for with uuid() is not mainstream but quite
> the opposite -- the user introduces something that he does *not* want.
> Such case should be handled explicitly with the user actually spelling
> out what he means like
>
> Foo::Foo()
> :
>     uuid_(boost::uuid::null()) // initially invalid {}

I do believe that a null uuid is a valid uuid, but regardless, if the 
common use case really is as you say, then sure lets have the default 
constructor call a function to generate a unique uuid.  And still 
provide a way to create a null uuid.

My work also heavily relies on uuids, but we almost never create one 
ourselves in code.  We get almost all our uuids from the database 
that our program uses.  I would be surprised if there aren't many 
others that use uuids in the same way, they receive them, they don't 
generate them.

> Best, Vladimir.

Regards,
Andy Tompkins
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

David Abrahams | 1 Dec 18:30
Favicon
Gravatar

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


on Mon Dec 01 2008, "Andy Tompkins" <atompkins-AT-fastmail.fm> wrote:

> On Sat, 29 Nov 2008 00:14:47 +1100, "Vladimir Batov"
> <batov <at> people.net.au> said:
>> Andy,
>>
<schnipp>

> Regards,
> Andy Tompkins

Vladimir raised some very compelling arguments, but IMO Andy answered
all of them well enough to allay any concerns I had about accepting this
library.  I'm curious whether Vladimir is satisfied with the response,
though.  Vladimir?

--

-- 
Dave Abrahams
BoostPro Computing
http://www.boostpro.com
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

Hartmut Kaiser | 2 Dec 01:57

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

> > On Sat, 29 Nov 2008 00:14:47 +1100, "Vladimir Batov"
> > <batov <at> people.net.au> said:
> >> Andy,
> >>
> <schnipp>
> 
> > Regards,
> > Andy Tompkins
> 
> Vladimir raised some very compelling arguments, but IMO Andy answered
> all of them well enough to allay any concerns I had about accepting
> this
> library.  

Is that a formal 'accept'?

> I'm curious whether Vladimir is satisfied with the response,
> though.  Vladimir?

Me too.

Regards Hartmut

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

Steven Watanabe | 26 Nov 05:05

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

AMDG

Andy Tompkins wrote:
>>  * In Representation: Is there a reason for a .to_string() member
>>    function instead of an operator std::string()? I.e., it reduces
>>    std::string s =
>> u.to_string() to std::string s( u );
>>     
>
> I tend to avoid implicit conversions.  One can also use
> boost::lexical_cast<std::string>(uuid).  Again, if this list wants
> operator std::string() and the like, I will add it.
>   

I don't think an implicit conversion here would be a good idea.

In Christ,
Steven Watanabe

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

Andy Tompkins | 27 Nov 05:57
Favicon

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

On Tue, 25 Nov 2008 20:05:35 -0800, "Steven Watanabe"
<watanabesj <at> gmail.com> said:
> AMDG
>
> Andy Tompkins wrote:
> >>  * In Representation: Is there a reason for a .to_string() member
> >>    function instead of an operator std::string()? I.e., it reduces
> >>    std::string s =
> >> u.to_string() to std::string s( u );
> >>
> >
> > I tend to avoid implicit conversions.  One can also use
> > boost::lexical_cast<std::string>(uuid).  Again, if this list wants
> > operator std::string() and the like, I will add it.
> >
>
> I don't think an implicit conversion here would be a good idea.

I agree.

>
> In Christ, Steven Watanabe
>

Regards,
  Andy Tompkins
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

Dave Jenkins | 25 Nov 23:27
Favicon

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

> - What is your evaluation of the design?

I like the design, but wish it had time-based UUIDs.

> - What is your evaluation of the implementation?

I had no problem reading the code.  But there's almost nothing in 
"uuid.ipp".  Could you merge it into "uuid.hpp" for simplicity?

> - What is your evaluation of the documentation?

I'd like to see the Rationale section come first, before you give the whole 
Class Synopsis. That would make it easier for casual readers to understand 
the purpose of the library before reading further.

A small example program near the beginning of the document would help too.

There is a spelling error in the Rationale section, s/indended/intended/.

> - What is your evaluation of the potential usefulness of the library?

It's a nice addition to Boost.

> - Did you try to use the library?  With what compiler?  Did you have any 
> problems?

Yes, I tried running some test code and had no problems.

I vote Yes, to include the UUID library in Boost.

Regards,
Dave Jenkins 
Andy Tompkins | 27 Nov 06:06
Favicon

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

On Tue, 25 Nov 2008 17:27:42 -0500, "Dave Jenkins"
<david <at> jenkins.net> said:
> > - What is your evaluation of the design?
>
> I like the design, but wish it had time-based UUIDs.

Understood.  I will look into this again.

> > - What is your evaluation of the implementation?
>
> I had no problem reading the code.  But there's almost nothing in
> "uuid.ipp".  Could you merge it into "uuid.hpp" for simplicity?

I could.  The idea was to put non-template functions in uuid.ipp so that
one could compile it in a .cpp to create a library.  I agree that there
is little in it.  I should have moved the definition of many uuid member
function to uuid.ipp.  But I think you are right, I should just merge
uuid.ipp into uuid.hpp.

> > - What is your evaluation of the documentation?
>
> I'd like to see the Rationale section come first, before you give the
> whole Class Synopsis. That would make it easier for casual readers to
> understand the purpose of the library before reading further.

Good idea.  I'll do this.

> A small example program near the beginning of the document would
> help too.

Sure.  I'll do this.

> There is a spelling error in the Rationale section,
> s/indended/intended/.

Thanks.

> > - What is your evaluation of the potential usefulness of the
> >   library?
>
> It's a nice addition to Boost.
>
> > - Did you try to use the library?  With what compiler?  Did you have
> >   any problems?
>
> Yes, I tried running some test code and had no problems.
>
> I vote Yes, to include the UUID library in Boost.
>
> Regards, Dave Jenkins
>

Thanks,
  Andy Tompkins
Steven Watanabe | 27 Nov 23:42

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

AMDG

Hartmut Kaiser wrote:
> The mini-review of Andy Tompkins UUID library starts today, November 23rd
> 2008, and will end on November 30th. 
> I really hope to see your vote and your participation in the discussions on
> the Boost mailing lists!
>   

I only saw a couple of issues:

uuid::get_showbraces_index() is not reentrant
(there is a race the first time it is called.)

operator<< doesn't handle setw.
operator>> ignores spaces.  Is this intended?

In Christ,
Steven Watanabe
Andy Tompkins | 29 Nov 05:58
Favicon

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

On Thu, 27 Nov 2008 14:42:19 -0800, "Steven Watanabe"
<watanabesj <at> gmail.com> said:
> AMDG
>
> Hartmut Kaiser wrote:
> > The mini-review of Andy Tompkins UUID library starts today, November
> > 23rd 2008, and will end on November 30th. I really hope to see your
> > vote and your participation in the discussions on the Boost mailing
> > lists!
> >
>
> I only saw a couple of issues:
>
> uuid::get_showbraces_index() is not reentrant (there is a race the
> first time it is called.)

I would like to fix this, but I'm not yet sure how.

> operator<< doesn't handle setw.
> operator>> ignores spaces.  Is this intended?

I intended to follow standard practices.  I forgot setw.  I will
fix this.  I didn't realize that operator>> ignores spaces.  I will
fix this too.

> In Christ,
> Steven Watanabe

Thanks,
  Andy Tompkins
muhammadchang | 3 Dec 06:21

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


Hi boosters,

First of all, my opinion is that this kind of classes, sometimes called
concrete types, are the very plus plus suggested by the language's name. A
well designed user type behaves and cost, exactly the same as any built in
type. This makes C++ unique in the plethora of available languages.
That said, I must mention that I am aware of the flaws pointed out by many
reviewers, specially those concerning interface. In particular, I don't like
the "default null constructor". For me, the default constructor MUST
generate a valid object, period. I have already copied/used Andy's code,
which I luckily found in your vault, and one thing I changed was this
default constructor [uuid_t uuid; // generates a valid random uuid]. If the
user is short of resources, she must use the copy constructor [uuid_t
nil(uuid_t::null());]. But I don't provide a bool is_null() const; method,
for operator==() does the job pretty well.
Despite this weaknesses, I think the library is ready for out-of-the-box
usage, so my vote is YES, this library should be part of boost.

Sincerely,
Kenneth

--

-- 
View this message in context: http://www.nabble.com/-Review--UUID-library-%28mini-%29review-starts-today%2C-November-23rd-tp20652849p20807056.html
Sent from the Boost - Users mailing list archive at Nabble.com.
Scott McMurray | 3 Dec 07:11

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

On Wed, Dec 3, 2008 at 00:21, muhammadchang <kennethlaskoski <at> gmail.com> wrote:
>
> First of all, my opinion is that this kind of classes, sometimes called
> concrete types, are the very plus plus suggested by the language's name. A
> well designed user type behaves and cost, exactly the same as any built in
> type. [...]  In particular