[Pharo-project] CRC in System-Hashing?
henrik.s.johansen at veloxit.no
Fri Mar 2 03:46:15 EST 2012
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.
More information about the Pharo-dev