On 09.06.2012 21:30, Dmitry Olshansky wrote:
The review process stalled long enough, let's kick start it with a small
yet a valuable module that was there for quite some time.
Ok, let's grill it a bit ;)
From browsing the docs alone:
ParserException - too general name likely to collide with user code,
call it UuidException instead
InsufficientInputException - maybe merge it with ParserException, just
add small <reason> field
Why? - Typical user can't do much with this extra info. By the end of
day an illegal UUID string is an illegal UUID string.
Add a constructor that takes variadic ubyte[] arr.../byte[] arr.. that
should just assert that argument has 16 bytes to hold UUID. (all these
casts in docs ...)
isNil - can we follow the precedent and use 'empty' for "has no useful
state" as does for instance std.regex.Regex and all of Range API.
Variant - may be confusing with as I thought of std.Variant till the
bitter end :)
uuidVersion - sadly we can't fix it's name ;)
nilUUID should be a property that returns UUID.init or better an enum,
no need to clutter object file with _TLS_ globals.
parseUUID - like Walter pointed out, could use input range
BTW functions taking a namespace UUID can take const UUID.
extractRegex ---> uuidRegex what's easier from user standpoint?
Now the code:
line 587: swap surely could do better then this:
swapRanges(this.data[], rhs.data[]);
e.g. swap it as 4 uints or 2/4 size_ts. Main reason for built-in swap is
efficiency. As it is now it won't surprise me if std.swap would be
faster with its 3 bit-blits.
line 1011: overload with no args
Creating Random generator on each call is unacceptable. It's better to
either remove it or use thread-local _static_ RNG here.
--
Dmitry Olshansky
