Review: std.uuid

Dmitry Olshansky dmitry.olsh at gmail.com
Sun Jun 10 02:02:16 PDT 2012


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


More information about the Digitalmars-d mailing list