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
>
>
>
