On 8/22/12 8:36 AM, Dmitry Olshansky wrote:
The discussion around new unified API for digest hash functions has
subdued, just in time as the review period has ended.
The voting for std.digest package starts today and ends on 29 August.
Rules are simple: reply in this thread with definite "YES" or "NO" on
whether this package should go into Phobos. Including descriptive short
notes is appreciated (esp. the NO ones).
Latest locations of docs and source:
Code: (location changed!)
https://github.com/jpf91/phobos/tree/newHash/std/digest
https://github.com/jpf91/phobos/compare/master...newHash
Docs: (location changed!)
http://dl.dropbox.com/u/24218791/d/phobos/std_digest_digest.html
http://dl.dropbox.com/u/24218791/d/phobos/std_digest_md.html
http://dl.dropbox.com/u/24218791/d/phobos/std_digest_sha.html
http://dl.dropbox.com/u/24218791/d/phobos/std_digest_crc.html
I couldn't make time for a review during the window, apologies.
Nevertheless I'd like to make a pass in hope that a few issues can be
fixed before the code is publicly released.
* No need for start() after default construction in the documentation
example of CRC32. BTW the documentation of start() suggests that it
_must_ be called even right after default construction, is that the case?
* In the template interface, no need for void peek(ref ...). Returning
by value is fine due to the RVO. Same about finish() - just keep the
functions returning by value.
* This example:
copy(oneMillionRange, &ctx); //Note: You must pass a pointer to copy!
suggests we're doing something wrong. I think a better solution would be
to have copy() take the target range by "auto ref", and institute this
passing convention as a general rule for output ranges.
* s/digestType/DigestType/
* s/isDigestable/isDigestible/
* s/startDigest/makeDigest/ - but then again I wonder if requiring
start() after default construction is needed.
* s/asArray/asStaticArray/. Better yet, we should include this primitive
in std.conv. Then we get to use to!(ubyte[16])(dynarray).
* The fact that digests aren't unique is trivial by the pigeonhole
principle, and should not be listed as a bug (the BUGS: tag implies bugs
in the implementation). It's more interesting if a document can be
altered to produce a specific digest. If that's what you meant, include
a link to the appropriate work.
* I don't think the aliases for WrapperDigest!Xyz should be defined.
Just call that Digest!Xyz and have people use it. It's about as short as
the aliases.
I vote for inclusion whether or not the points above are addressed, but
of course it would be great if they were minded before release.
Apologies for the late review.
Andrei
