Re: Net_Vpopmaild

Joe Stump wrote: > Some initial feedback: > > 1.) It seems everything is public. I'd think $log, for instance, could > be private. I don't see a reason why anything would need to log > outside of this package. Additionally, it might make more sense to > implement an accept() method so that I can pass my own non-file Log > object to Net_Vpopmail for logging. > > 2.) All if/else, etc. must have { } e.g. one line if statements don't > conform to PEAR CS. It must be: > > if (true) { > return true; > } > > 3.) I'd highly recommend using preg_* instead of ereg. I'm not sure if > this is explicitly stated in PEAR CS, but php.net widely considers > ereg to be outdated. > > 4.) PHP5 packages cannot use PEAR_Error and must use exceptions. > Create Net_Vpopmail_Exception extending PEAR_Exception and throw that > instead of PEAR_Error. > > 5.) I'm fairly sure Net_Socket and the Mail package you use are PHP4. > PHP5 PEAR packages cannot rely on PHP4 packages per the RFC's. > > 6.) In setLimits() I'd make those arrays static so they aren't created > in memory for each time the function is called. > > 7.) Instead of validEmailAddress() maybe use Validate::email() ? Not > sure if that's RFC compliant though. > > 8.) Nitpicky, but I don't see any .phpt or unit tests. > > For a first time package, though, this looks good and definitely > serves an awesome purpose. Thanks! > > --Joe I've put together a 0.0.1 package that addresses all of the above, except for unit tests. The updated API can be viewed here: http://shupp.org/Net_Vpopmaild/ And the package can be downloaded and installed via pear here: http://shupp.org/Net_Vpopmaild-0.0.1.tgz Any further comments are welcome. Otherwise, I'll move forward with submitting it. Thanks! Bill Shupp P.S. Joe Stump, I used your simple Framework_Exception class, and just renamed it Net_Vpopmaild_Exception, and left you as the author. Please let me know if you have any issue with that...