std.uuid is ready for review

Johannes Pfau nospam at example.com
Thu Feb 9 01:57:21 PST 2012


Thanks for your feedback! Comments below:

Am Wed, 08 Feb 2012 23:40:14 -0600
schrieb "Robert Jacques" <sandford at jhu.edu>:

> 
> 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.
done

> 
> 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.
done

> 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."
This was a leftover from boost, fixed.

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

> And shouldn't use writelns. i.e.
>   assert(entry.uuidVersion == UUID.Version.nameBasedSha1);
ok. I had to rewrite the example, but the writelns are gone now

> 
> 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 makes sense and looks better. I don't have an opinion about
the other functions, but struct as namespace vs free functions
has always led to debates here, so I'm not sure if I should change it.
I need some more feedback here first. (Also imho randomUUID() looks
better than UUID.random(), but maybe that's just me)

> 
> UUID.nil should be an alias/enum to UUID.init, not an immutable.
alias UUID.init nilUUID;
doesn't work, it would work if nil was a member of UUID, but see above
for comments on that.
Made it an enum for now.

> 
> There's an additional toString signature which should be supported.
> See std.format.
You're talking about this, right?
const void toString(scope void delegate(const(char)[]) sink);

Nice, when did the writeTo proposal get merged? I must have totally
missed that, actually writeTo is a way better choice here, as it can
avoid memory allocation.

but it seems to!string doesn't support the new signature?

BTW: How should sink interact with pure/safe versions? Can't we just
change that declaration to?

const @safe [pure] void toString(scope @safe pure void
delegate(const(char)[]) sink);

> 
> uuidVersion() -> ver()?
I'm not sure, uuidVersion is indeed quite long, but it is more
descriptive as ver



More information about the Digitalmars-d mailing list