On 24-Jun-12 19:23, Johannes Pfau wrote:
There are still some open questions:
OOP interface: Digest.finish()
This can only throw if the supplied buffer is too small. Make this
nothrow & throw an Error on too small buffer? Or check buffer only in
debug mode using asserts or preconditions?
Yup. I'd suggest to simply assert on it.
CRC32:
The current implementation doesn't seem to be compliant
to the 'common' CRC-32-IEEE 802.3 form, at least it doesn't pass these
test vectors:
http://www.febooti.com/products/filetweak/members/hash-and-crc/test-vectors/
http://www.lammertbies.nl/comm/info/crc-calculation.html
http://rosettacode.org/wiki/CRC-32
I believe there ought to be CRC32 with *any* kind of license on the web.
I'll throw in some things that looked plain wrong to me:
- calculateDigestOOP. Besides having awful, awful name it really should
be final method of Digest interface (yes, we have these since quite some
time)
- digestToString helper. What kind of string? Why not just toString as
member? Or rather clarify that it obtains hexadecimal representation of
digest.
e.g. toHexString looks far more intuitive for me (again check out
toString debate - I hardly believe that hashes have only one possible
string representation)
- calculateDigest (also called calculateHash somewhere) - why not just
digest ? In general I'm weary and tired of no-brainer prefixes. They add
extra symbols for no benefit, because, of course, digest is calculated.
And so does sum for instance, yet we (would) have sum in algorithm not
calculate sum, same for min/max etc.
(I think one of hardest goals of std.* is to meet the best balance of
clarity and brevity.)
- same goes for md5Sum --> md5Of (i'd love to do plain md5 but maybe
it's much)
- crc32Sum --> scr32Of //basically the fact that both are sums helps
very little, yet the suffix 'Of' (I think) indicates that the
calculation is to happen right here (i.e. that it's not initialization
or smth).
Everything else looks good, though docs may need some proof-reading.
Thanks for pushing this proposal.
--
Dmitry Olshansky
