[Pharo-project] CRC in System-Hashing?
maxleske at gmail.com
Fri Mar 2 04:21:19 EST 2012
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
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.
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the Pharo-dev