Comments for "Crypt_FSHP"

» Submit Your Comment
Please log in to enter your comment. If you are not a registered PEAR developer, you can comment by sending an email to
» Comments
  • Michael Gauthier  [2009-07-03 03:52 UTC]

    Great proposal. Some suggestions:

    1.) Run your code through phpcs to pick up minor whitespace and coding style changes.

    2.) CC public domain is not a recognized PEAR license. Consider using the BSD license (or MIT, Apache2).

    3.) Your proposal has great API documentation. It could be improved by adding documentation for return values. Method docs should be second-person imperative ("Generates a hash" rather than "Generate a hash").

    4.) Parameter names should be full words (personal preference, not required by pear).

    5.) The parameter order may be improved. I can see specifying the salt length and iteration count before specifying an actual salt value. Having to specify the salt value as null could be annoying.

    6.) The $variant parameter uses magic numbers. Consider using strings ('sha1', 'sha256', etc) instead. Magic strings are easier to read and to remember than magic numbers.

    7.) Exception parameters are not documented.

    8.) Consider providing a method to get the unsupported variant from an UnsupportedVarientException,

    9.) Consider an abstract base exception class Crypt_FSHP_Exception so users can capture any exception the package may throw.

    10.) Exception class should be defined in a separate file.

    11.) Methods are called statically in but are not defined statically. This will cause a warning in PHP. Either define methods as static, or call using $this.

    12.) The 'hash' extension should be specified as a requirement in package.xml. Even though it is built by default in PHP 5.1.2, it is possible to disable.

    13.) Since the API is so small, more extensive tests could be written.
  • Michael Gauthier  [2011-01-30 21:58 UTC]

    Berk, are you still working on this proposal? Did you get a chance to review my suggestions?
  • Berk D. Demir  [2011-08-29 21:44 UTC]

    FSHP is dead in favor of memory hard algorithms which can provide more realistic password security in 2011.