[PEPr] Comment on Mail::Mail_CheckUser

I find your class interesting, though I'm too interested what happens when the mailserver doesn't support the lookup. Some more feedback on the code: a) It would be nice if you relied on $_ENV as well -- depending on the sapi used. E.g. in a CLI script, there's no $_SERVER. Afaik, $_SERVER comes from the webserver. If there is none, then you're out of luck. It also makes sense to double-check everything you are using there because there are no guarantees on what is provided. You can check "where" your class is invoked with php_sapi_name(). b) Please change the license. PHP licensed packages are not accepted anymore. Instead we suggest BSD, MIT, LGPL, etc.. c) I'd push all your public vars into a protected array $data (or something), and offer set*() and get*() methods accordingly to maybe validate the set process, etc.. e.g. public function setTimeout($timeout) { $this->data['timeout'] = (int) $timeout; return $this; } Generally get*() and set*() are a good idea so people can modify the object on the run and don't have to re-instantiate a new class each time. Especially if you run this in bulk this would be interesting. d) Many of the methods in Net_SMTP return PEAR_Error objects. It wouldn't hurt to collect them in case something goes wrong to make at debugging a little easier. IMHO, a simple true/false is just not enough. Maybe it would be good to re-cast the PEAR_Error object to an exception and throw it. But that depends how severe a single failure (inside a loop) is. I think you could at least collect them and if all failed, throw an exception and allow people to retrieve them. e) Is there no way to re-use the instance of Net_SMTP inside the loop? (Just curious.) f) Run phpcs (http://pear.php.net/package/PHP_CodeSniffer) to fix some minor issues. g) It would be nice to let tests run without connection to a server, or maybe it could be "mocked"? h) Maybe refactor the checkEmail() method -- it's kind of long. :-) Also, great job on rolling a package already, and also kudos on the tests! Till -- http://pear.php.net/pepr/pepr-proposal-show.php?id=605