View mode: basic / threaded / horizontal-split · Log in · Help
February 07, 2012
Re: std.uuid is ready for review
On 02/02/2012 01:26 PM, Johannes Pfau 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.
>
>
>

Vote + (yes, works fine)

I want it and I need it. A Databases Access Layer (may it be a full 
blown  ORM  or even "just" a (D)ata (A)ccess (L)ayer ) depends on having 
unique IDs. ?

A convenience wish : string OID = GetUUID(); // RA UUID
February 08, 2012
Re: std.uuid is ready for review
Am Tue, 07 Feb 2012 00:00:26 +0100
schrieb Piotr Szturmaj <bncrbme@jadamspam.pl>:

> Johannes Pfau wrote:
> > Code: https://github.com/jpf91/phobos/blob/std.uuid/std/uuid.d
> > API-Docs: http://dl.dropbox.com/u/24218791/d/src/uuid.html
> 
> <snip>
> @trusted UUID shaUUID(const(char[]) name, UUID namespace = nilUUID);
> @trusted UUID shaUUID(const(ubyte[]) data, UUID namespace = nilUUID);
> </snip>
> 
> When casting from void[] to ubyte[] will be working in CTFE (if
> ever), you might use just one function taking const(void[]) and cast
> it to ubyte[] internally.
I'm not sure, most of the time you'd use the string version anyway and
in the few cases when you really want to use raw data a cast to ubyte[]
doesn't seem that bad.

> About name, I think it should be sha1UUID.

done
February 08, 2012
Re: std.uuid is ready for review
Am Tue, 07 Feb 2012 06:54:49 -0800
schrieb bls <bizprac@orange.fr>:

> On 02/02/2012 01:26 PM, Johannes Pfau 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.
> >
> >
> >
> 
> Vote + (yes, works fine)
> 
> I want it and I need it. A Databases Access Layer (may it be a full 
> blown  ORM  or even "just" a (D)ata (A)ccess (L)ayer ) depends on
> having unique IDs. ?
> 
> A convenience wish : string OID = GetUUID(); // RA UUID
> 

Sorry, I don't understand that example. What should GetUUID() return? a
Random UUID? What is the difference to randomUUID() then?
February 08, 2012
Re: std.uuid is ready for review
Am Sun, 05 Feb 2012 21:48:36 -0800
schrieb Walter Bright <newshound2@digitalmars.com>:

> On 2/2/2012 1:26 PM, Johannes Pfau wrote:
> > 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.
> 
> I'd like to see a string constant that is a regex for a UUID in its
> canonical form. The user could then simply pass that string to
> std.regex and pick UUIDs out of text.

done
February 09, 2012
Re: std.uuid is ready for review
On Thu, 02 Feb 2012 15:26:48 -0600, Johannes Pfau <nospam@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()?
February 09, 2012
Re: std.uuid is ready for review
On 2/8/2012 9:38 AM, Johannes Pfau wrote:
> Am Sun, 05 Feb 2012 21:48:36 -0800
> schrieb Walter Bright<newshound2@digitalmars.com>:
> done

danke
February 09, 2012
Re: std.uuid is ready for review
Thanks for your feedback! Comments below:

Am Wed, 08 Feb 2012 23:40:14 -0600
schrieb "Robert Jacques" <sandford@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
February 10, 2012
Re: std.uuid is ready for review
On Thu, 09 Feb 2012 03:57:21 -0600, Johannes Pfau <nospam@example.com> wrote:
> Thanks for your feedback! Comments below:
> Am Wed, 08 Feb 2012 23:40:14 -0600
> schrieb "Robert Jacques" <sandford@jhu.edu>:

[snip]

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

Hmm... I'd agree that randomUUID reads better than UUID.random. IMO well named free-functions are generally better than fake namespaces via structs. However, fake namespaces via structs a generally better than fake namespaces via free-function naming convention (i.e. [function][namespace] or [namespace][function]. That said, I think the bigger problem is that all these functions are effectively constructors. I'd suspect that overloading UUID(...) would be a clearer expression of the concepts involved. As for syntax, maybe something like: UUID(Flag!"random", ... ) to disambiguate when necessary.

[snip]

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

I missed it to, then I saw code using it and smiled.

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

I think that's worthy of a bug report.

> 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);

Since the to!, etc. are all templated, adding extra attributes is okay.

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

Shrug. One's long, one's short, neither is perfect, version is a keyword.
February 10, 2012
Re: std.uuid is ready for review
On 2012-02-10 06:48, Robert Jacques wrote:
> On Thu, 09 Feb 2012 03:57:21 -0600, Johannes Pfau <nospam@example.com>
> wrote:
>> Thanks for your feedback! Comments below:
>> Am Wed, 08 Feb 2012 23:40:14 -0600
>> schrieb "Robert Jacques" <sandford@jhu.edu>:
>
> [snip]
>
>>> 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)
>
> Hmm... I'd agree that randomUUID reads better than UUID.random. IMO well
> named free-functions are generally better than fake namespaces via
> structs. However, fake namespaces via structs a generally better than
> fake namespaces via free-function naming convention (i.e.
> [function][namespace] or [namespace][function]. That said, I think the
> bigger problem is that all these functions are effectively constructors.
> I'd suspect that overloading UUID(...) would be a clearer expression of
> the concepts involved. As for syntax, maybe something like:
> UUID(Flag!"random", ... ) to disambiguate when necessary.

UUID(Flag!"random", ... ) is just ugly. It's far better to use a static 
function with a descriptive name.

-- 
/Jacob Carlborg
February 10, 2012
Re: std.uuid is ready for review
On Friday, February 10, 2012 09:56:36 Jacob Carlborg wrote:
> On 2012-02-10 06:48, Robert Jacques wrote:
> > On Thu, 09 Feb 2012 03:57:21 -0600, Johannes Pfau <nospam@example.com>
> > 
> > wrote:
> >> Thanks for your feedback! Comments below:
> >> Am Wed, 08 Feb 2012 23:40:14 -0600
> > 
> >> schrieb "Robert Jacques" <sandford@jhu.edu>:
> > [snip]
> > 
> >>> 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)
> > 
> > Hmm... I'd agree that randomUUID reads better than UUID.random. IMO well
> > named free-functions are generally better than fake namespaces via
> > structs. However, fake namespaces via structs a generally better than
> > fake namespaces via free-function naming convention (i.e.
> > [function][namespace] or [namespace][function]. That said, I think the
> > bigger problem is that all these functions are effectively constructors.
> > I'd suspect that overloading UUID(...) would be a clearer expression of
> > the concepts involved. As for syntax, maybe something like:
> > UUID(Flag!"random", ... ) to disambiguate when necessary.
> 
> UUID(Flag!"random", ... ) is just ugly. It's far better to use a static
> function with a descriptive name.

Agreed.

- Jonathan M Davis
1 2 3
Top | Discussion index | About this forum | D home