Thanks for your feedback! Comments below: Am Wed, 08 Feb 2012 23:40:14 -0600 schrieb "Robert Jacques" <sandf...@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
