Boost logo

Boost :

Subject: Re: [boost] [Review] UUID library (mini-)review starts today, November 23rd
From: Vladimir.Batov_at_[hidden]
Date: 2008-11-26 16:09:01


*** What is your evaluation of the design?

Unfortunately I have serious reservations about the interface. Like,

1. "uuids" namespace seems unnecessary and burdensome. The generator
classes
can be introduced inside boost::uuid.
2. handling "char const*", "wchar_t const*", std::basic_string-based
arguments should be templated/generalized.
3. Using a separate "generator" class to create a new UUID seems
unorthodox
as I feel the standard way of calling a constructor would be more inline
with the standard C++ practices. Like

    boost::uuids::basic_uuid_generator<boost::mt19937> gen2;
    boost::uuid id(gen2);

instead of

    boost::uuids::basic_uuid_generator<boost::mt19937> gen2;
    boost::uuids::uuid u2 = gen2();

That way

    boost::uuid id; // Works as Linux uuid_generate();
    boost::uuid id(gen2); // Works as Linux uuid_generate_random();

4. The default uuid() creating a "null" UUID is too subtle, wide open for
misinterpretation and in fact counter-intuitive. For example, my (and many

others that I know) interpretation of that constructor is like Linux
uuid_generate() was called behind it. The creation of an invalid/null UUID

needs to be explicitly stated with something like

    boost::uuid id = boost::uuid::null();
    boost::uuid id = boost::uuid::invalid();

then "is_null()" method would be somewhat expected/justified.

5. I'd expect to be able to use "if (!uuid) then..." rather than "if
(uuid.is_null()) then...".
6. to_wstring() does not seem to belong to the class. I understand it is
conveniend but I fdo not feel that is core UUID functionality and,
therefore, should (and can) be handled by other means.

*** What is your evaluation of the implementation?

I am OK with the implementation with a few comments.

1. Unfortunately, it does not seem to provide time-based construction. I
read explanations why not but the usefulness of such a simple concept very

much depends on its completeness.
2. I was surprised to see NULL used and postincrements.
3. A lot of raw numbers around the code.
4. I am very suspicious of using std::streams as from what I know they are

not exactly fast.

*** What is your evaluation of the documentation?

Well, in all honesty I do not feel like the uuid.html file supplied with
the
package (uuid_v13.zip) can be considered as adequate documentation.
Explanations seem scetchy. Links seem broken (like "see operators.hpp"
points to a non-existing "operators.htm").

*** What is your evaluation of the potential usefulness of the library?

I certainly think such a facility needed.

*** Did you try to use the library? With what compiler? Did you have any

problems?

Tried with MSVC8 with results similar to already reported. Iseems like at
least some of the warnings (like "warning C4244: '=' : conversion from
'int'
to 'char', possible loss of data") need to be addressed.

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

I spent a couple of hours with it.

*** Are you knowledgeable about the problem domain?

Well, I consider myself a "knowledgeable user". I certainly will not be
able
to claim to be intimately familiar with the generation algorithms.

*** Conclusion:

The UUID concept is most certainly very useful and such a facility is an
essential gadget in everyone's toolbox. However, there are many
implementations around (OpenLDAP, Linux, Windows to name a few). For
boost::uuid to be accepted and used instead other alternatives it needs to

be as complete as its competitors and to stand out -- to be superior in
interface, convenience or something. In a couple of hours I spent with
boost::uuid I unfortunately did not manage to get that impression.
Therefore, I am reluctantly voting against its acceptance in its present
form.

Thanks,
Vladimir.


Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk