Am Mon, 25 Jun 2012 00:02:00 +0400 schrieb Dmitry Olshansky <dmitry.o...@gmail.com>:
> 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. OK > > > > > 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) OK. > - 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) toString as a member doesn't work well for some hashes. Those hashes destroy the internal state in finish(). So after a toString() call the hash would be either invalid or reset to initial state, which is counterintuitive. But toHexString sounds like a good solution. (digestToString was the name used in std.md5, btw) > > - 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.) OK, I'll rename it to digest. > > - 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). OK. those were originally called 'sum', but I think it's common to use multiple hash modules at the same time so name clashes would happen. md5Of/crcOf sound great. > > Everything else looks good, though docs may need some proof-reading. > Thanks for pushing this proposal. Yep that was only a first draft. I still need to run it through a spell checker, proof read it and add some links.
