Proposal Auth_WIKID Extra Comment (QA)

Hello all, this is an email to do extra QA on http://pear.php.net/pepr/pepr-proposal-show.php?id=321 First of all Greg, good work on the package, I really like the overall structure, the code is very clean, and I like the comments themselves and it looks good. However, I must add a couple points: The package is noted as PHP5, so the variable properties should be respected. I see in the documentor's code that all the variables access is noted as @access private, therefore, you should usually make the variables private $_isConnected instead of var $_isConnected; Because by default var is now E_STRICT compliant, however it will make the variables public. Also, making the variables private is not recommended as it makes them less flexible, therefore it is recommended to use * protected for pear packages. Next point: In the constructor of your class (which is not defined using __construct() { }) I see that you are doing :

      $min_version = "5.0.0";
       if (version_compare($min_version, phpversion(), ">=")) {

           echo "Incompatible version - Auth_WiKID requires PHP >=

$min_version\n";

           return null;
       }

This is overhead because the pear installer is handling all this using the package.xml file (dependencies). Next point: In php5, you can use __destruct() in order to execute the destroy and all your close() function. Next point: As stated you have to make a dependency for openssl Next point: Nothing else to say else than good work for this quality package :) Thanks, P.S. Any other input would be appreciated guys.. (wink)(wink) -- David Coallier, Founder & Software Architect, Agora Production (http://agoraproduction.com) 51.42.06.70.18