Markus Tacker [2009-07-17 19:18 UTC] Why, oh why, PHP4?
The applicability of this class is limited as this type of email address verificiation does not work with many mail servers. The common and most established way to do that is by double opt-in.
There are not methods to set sender and fqdn or to access the result (see first paragraph).
Greg Beaver [2009-07-17 19:27 UTC] use __construct() for the constructor
have the sender and fqdn as constructor parameters, default value null, and only if not passed in should they set themselves from $_SERVER.
PEAR is no longer accepting PHP4 packages, this must work in PHP 5 with no E_STRICT/E_DEPRECATED warnings. Fortunately, just change the constructor name and you're good to go (on a cursory exam of the source)
Takayuki Miyauchi [2009-07-17 19:55 UTC] Hello Markus and Greg, thanks to comments.
Net_SMTP and Mail is PHP4.
I misunderstands, this packege must developped by same version with these packages.
I will soon change to PHP5.
Which do you think is good?
function __construct($fqdn=null, $sender=null)
{
if ($fqdn) {
$this->fqdn = $fqdn;
}else{
................
}
}
or
function __construct($fqdn, $senderl)
{
$this->fqdn = $fqdn;
$this->sender = $sender;
}
Markus say:
"The common and most established way to do that is by double opt-in."
Sorry, i can't understand double opt-in.
please teach me it.
I'm not good at English,
If I make mistakes in my English,please pardon me.
Till Klampaeckel [2009-07-18 10:51 UTC] 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
Alan Knowles [2009-07-19 00:49 UTC] Naming:
I'd be tempted to put this in Validation - Validate_MailViaSmtp seems a bit more specific on what this package does.
Failure:
The only 'fail condition of this package should really be recieving 5** response, I think almost every other situation indicates 'possible' success (eg. greylisted servers, tcp delay connect servers etc.)
Takayuki Miyauchi [2009-07-19 03:38 UTC] Thanks to comments Till and Alan.
I wish to express my gratitude for very getting the comments that becomes reference.
Most of the content pointed out will be reflected at once.
"Validate_MailViaSmtp"
This is very nice name.
I want to hear more a lot of opinions.
(I gave a name referring to Perl CPAN library Mail::CheckUser. )
|
