Re: Net_Vpopmaild

On 7/13/07, Bill Shupp <hostmaster@shupp.org> wrote:

Joe Stump wrote: 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. Ok, that's an interesting idea. I figured you could just overwrite the constructor and instantiate Log with your own parameters. But accept() seems more flexible. 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;

} Ah, I still have the habit of not bracing single line conditions. I'll fix them. 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. RE: #4, is that for raising errors only? And is there a PHP5 version of Net_Socket underway? Or would I need to make Net_Vpopmaild PHP4 compliant to release it as stable? 6.) In setLimits() I'd make those arrays static so they aren't created in memory for each time the function is called. Ah, that's a good idea. 7.) Instead of validEmailAddress() maybe use Validate::email() ? Not sure if that's RFC compliant though.

Side not to Philippe: yes it is compliant.

Looks like the 'use_rfc822' option can make it RFC compliant. 8.) Nitpicky, but I don't see any .phpt or unit tests. Yeah, I was waiting for feedback before I delve into tests, examples, etc. For a first time package, though, this looks good and definitely serves an awesome purpose. Thanks! Thanks for the detailed feedback. Regards, Bill -- PEAR Development Mailing List (http://pear.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php

-- David Coallier, Founder & Software Architect, Agora Production (http://agoraproduction.com) 51.42.06.70.18