Comments for "HCE_SHA"

» Submit Your Comment
Comments are only accepted during the "Proposal" phase. This proposal is currently in the "Finished" phase.
» Comments

  • Philippe Jausions  [2008-01-01 19:18 UTC]

    1. Please provide direct access to the source, besides the packaged version.
    2. Don't use TAB but spaces to indent code
    3. Class should at least be named "Crypt_HCE_SHA"
    4. Is sha1 the only hash function supported? The thinking behind the question, is whether SHA should even be in the class name.
    5. Put spaces around = signs, after commas, and before { in for loops
    6. What is the purpose of the empty __destruct() method?
    7. Public methods should follow camelCase i.e. blockEncrypt() instead of hce_block_encrypt()
    8. Clean up formatting of doc blocks to match PEAR's coding standard
    9. Missing doc block for private members $secret_key and $random_key)
    10. I don't have much suggestions for better method names, but hce_block_encode_mime() and hce_block_decode_mime() but "mime" doesn't necessarily implies "base64", and "encode" vs "encrypt" is not very consistent.

  • Chuck Burgess  [2008-01-09 22:11 UTC]

    Coding standard concerns seem to be covered already. I also agree that I'd expect this kind of class to be in the Crypt hierarchy.

    The only thing that jumped out at me is how much hce_block_encrypt() and hce_block_decrypt() look like they're about 95% identical. I don't think I could avoid the temptation of refactoring all that common code into private method that both of those two public methods would utilize.