std.uuid is ready for review

Robert Jacques sandford at jhu.edu
Wed Feb 8 21:40:14 PST 2012


On Thu, 02 Feb 2012 15:26:48 -0600, Johannes Pfau <nospam at example.com> wrote:
> Hi,
>
> std.uuid is ready to be reviewed. As far as I know there's nothing
> being reviewed right now, so we could start the review as soon as
> a review manager has been found.
>
> About std.uuid (copied from the module documentation):
> ---------------------
> This is a port of boost.uuid from the boost project with some minor
> additions and API changes for a more D-like API. A UUID, or Universally
> unique identifier, is intended to uniquely identify information in a
> distributed environment without significant central coordination. It
> can be used to tag objects with very short lifetimes, or to reliably
> identify very persistent objects across a network. UUIDs have many
> applications. [...]
> ---------------------
>
> Code: https://github.com/jpf91/phobos/blob/std.uuid/std/uuid.d
> API-Docs: http://dl.dropbox.com/u/24218791/d/src/uuid.html
>
> Note: The code and documentation for shaUUID has already been written,
> but until phobos has support for SHA1, that can't be included. The code
> is currently commented out in the source file (it's well tested
> with some 3rd party SHA1 code), but the documentation for those
> functions is included in the API-docs. I think those functions should
> be reviewed as well, so that it's possible to add them to phobos with a
> simple pull request at a later date.
>
> Note2: std.uuid also need this pull request:
> https://github.com/D-Programming-Language/phobos/pull/398
> It adds a isRandomNumberGenerator template to detect if a template
> parameter is a random-number generator type.

Comments in order of generation, not importance:

"This is a port of  boost.uuid from the boost project with some minor additions and API changes for a more D-like API." shouldn't be the first line in the docs.

"A UUID, or Universally unique identifier," one of these (or both) should link to the Wikipedia article.

Variant is the name of an existing Phobos library type and version is a D keyword. Now, thanks to Wikipedia I understand that variants and versions are a core part of UUIDs, but a lack of documentation explanation sent me for a loop. These terms should be explained better.

Suggested rewrite:
"This library implements a UUID as a struct allowing a UUID to be used in the most efficient ways, including using memcpy. A drawback is that a struct can not have a default constructors, and thus simply declaring a UUID will not initialize it to a value generated by one of the defined mechanisms. Use the struct's constructors or the UUID generator functions to get an initialized UUID."->
"For efficiency, UUID is implemented as a struct. UUIDs therefore default to nil. Use UUID's constructors or generator static members to get an initialized UUID."

Also, this snippet needs to be part of the introductory example.
  UUID id;
  assert(id.isNil);
Oh, and the example should be fully commented. i.e.
  assert(id.isNil); // UUIDs default to nil
And shouldn't use writelns. i.e.
  assert(entry.uuidVersion == UUID.Version.nameBasedSha1);

All the generators have the function name [name]UUID. Instead, make these function static member functions inside UUID and remove the UUID from the name. i.e. nilUUID -> UUID.nil randomUUID -> UUID.random., etc. I'm not sure if you should also do this for dnsNamespace, etc. (i.e. dnsNamespace -> UUID.dns) or not.

UUID.nil should be an alias/enum to UUID.init, not an immutable.

There's an additional toString signature which should be supported. See std.format.

uuidVersion() -> ver()?


More information about the Digitalmars-d mailing list