Sounds good to me. Now for 1.5 alpha :) Stef
On Mar 2, 2012, at 10:21 AM, Max Leske wrote: > Thanks for the responses. > > I'll wait a little longer and see if Rob has anything to say about this. > Otherwise I will do what Henrik suggests: > • deprecate SecureHashAlgorithm (why not remove it completely?) > • refactor users of SecureHashAlogrithm to use SHA1 instead > • add CRC to SystemHashing (possibly a copy from Cryptography) > • refactor the Zip implementation to use the new CRC class > • deprecate crc methods in Zip implementation > > I actually like #hashMessage: better than #hashStream:. #hashStream: says to > my "I expect a stream as my argument" where in reality it will take a string > or a ByteArray (neither of which is obviously a stream…). > What I found irritating though was that one has to send #new to > SecureHashAlgorithm first before hashing. So I propose to keep #hashMessage: > but to move it to the class side of SHA1: > > SHA1 class>>hashMessage: aStringOrByteArray > ^ self new > hashStream: aStringOrByteArray; > yourself > > Cheers, > Max > > > On 02.03.2012, at 09:46, Henrik Johansen wrote: > >> >> On Mar 2, 2012, at 9:05 AM, Marcus Denker wrote: >> >>> >>> On Mar 2, 2012, at 8:19 AM, Max Leske wrote: >>> >>>> I need to calculate CRC for Git and couldn't find it in the system >>>> packages (I guess I'd find it in Cryptography). Since Pharo has a system >>>> package called "System-Hashing" I was wondering if anyone sees a reason >>>> why CRC should not be part of that package. That would also allow the Zip >>>> implementation to use the system CRC function (at the moment Zip >>>> implements its own CRC check). >>>> >>> Yes, that would be nice. But people will yell at you for making Pharo >>> bloated and going in the completely wrong direction and >>> that you should and never ever add any code. >> >> Decomposing != Bloating. >> As long as you refactor the current users (which currently use the >> implementation in ZipWriteStream) to use the new CRC class, it should be >> fine. >> I would suggest deprecating the class method in ZipWriteStream rather than >> removing it though, based on the users in image, it is a likely method to be >> in use by external packages doing CRC calculations. >> >> It also seems the primitive for updating CRC is not reliant on the CrcTable >> class var, so should be safe to use in new class' method as well. >> >>> >>>> Another thing: going through Sytem-Hashing I noticed that there are two >>>> implementations for SHA1: "SHA1" and "SecureHashAlgorithm". They both have >>>> the exact same class comment but vary slightly in the messages they >>>> implement (although most of them are present in both classes). Can we >>>> remove one? >>> >>> Yes. But for sure someone will yell at you if you remove it, because the >>> system should be stable and not force people to >>> change their code. >> >> IIRC, it was initially included as SecureHashAlgorithm, being renamed when >> imported from the Cryptography package. >> To allow existing packages relying on Crypto to load cleanly, SHA1 was later >> also included. >> >> As there are already other name clashes with the Crypto package (MD5 for >> instance) I would vote for deprecating SecureHashAlgorithm. >> (SHA1 is arguably a more descriptive name anyways) >> Not sure if it's worth including hashMessage: in SHA1, it's just a utility >> method. >> >> Cheers, >> Henry >> >> >> >
