Re: Package proposal: HTTP_Status

From: Date: Tue, 26 Aug 2003 18:14:20 +0000
Subject: Re: Package proposal: HTTP_Status
References: 1 2 3 4  Groups: php.pear.dev 
Request: Send a blank email to pear-dev+get-20568@lists.php.net to get a copy of this message
Alexey Borzov wrote:
http://pear.php.net/package/HTTP_Header
It's wasteful to create a dependency on HTTP_Header if all you are going to do is use its constants. You can't even use any of its methods in other classes to process the status code, as it only sends the codes. Look at HTTP_Request:
    // Check for redirection
    if (    $this->_allowRedirects
        AND $this->_redirects <= $this->_maxRedirects
        AND $this->getResponseCode() > 300
        AND $this->getResponseCode() < 399
        AND !empty($this->_response->_headers['Location'])) {
How could you use any of HTTP_Header's constants to simplify that? You can't. But you could say:
    // Check for redirection
    if (    $this->_allowRedirects
        AND $this->_redirects <= $this->_maxRedirects
        AND HTTP_Status::isRedirection($this->getResponseCode())
        AND !empty($this->_response->_headers['Location'])) {
What's the use of another package, that is a wrapper around an array and a bunch of number comparisons?
We've been over this already. It's supposed to avoid the need to reinvent the wheel in every package that needs to translate HTTP status codes. There is nothing that says that a package needs to be really complex. As Stephan already said, "it centralizes some method, which is something I like and I could use it in HTTP_Server."
It sends everything HTTP_Request is able to send, and HTTP_Request is quite capable of sending a conditional GET. ;]
Ok, but there are easy ways of working around your "304 is not a redirection" problem. I'm not claiming to be an expert on all of the HTTP classes; it just seemed like having all of these error codes decentralized is asking for trouble.
if (!in_array($code, array(304, 307)) && HTTP_Status::isRedirection($code)) {
    ...
}
You call *that* simplification? :]
It's certainly better than your switch()! I was considering adding an optional parameter to all of the isXXXX() methods to let you pass an array of parameters, one of which would be codes to exclude (or include). For instance, you could call: HTTP_Status::isRedirection($code, array("exclude" => array(304, 307)));
BTW, there is a genuine error in your class. Method isError() reads: function isError($code) {
    return ($this->is_error_server($code) || $this->is_error_client($code));
} while is_error_server() and is_error_client() are not defined.
Sorry, missed that when I went through the manual's coding standards again and noticed the camelback naming scheme. It's fixed now. -- Marshall Roch

« previous php.pear.dev (#20568) next »