Sorry, I couldn't resist :$ On Fri, Mar 2, 2012 at 6:21 AM, Max Leske <[email protected]> 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 > > http://code.google.com/p/pharo/issues/detail?id=5427&thanks=5427&ts=1330699419 > > - 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 > > http://code.google.com/p/pharo/issues/detail?id=5426&thanks=5426&ts=1330699298 Guille > > 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 > > > > >
