Re: On private vars, inheritance and API design

Hi, Stefan Walk wrote:

1. Violating the "private" character of methods/properties It seems to be quite common to access private methods and properties in non-private context. A "pcregrep -ril '(?<!this)->_' * | grep -v test" in the pear directory listed 172 files - I know this catches some false positives too, so don't take the number as accurate. It's just there to show that it isn't an isolated case. Let me show you an example (taken from HTTP_Request): function getResponseBody() {

    return isset($this->_response->_body) ? $this->_response->_body : false;

} $this->_response is an instance of HTTP_Response. It isn't documented if _body is meant to be protected or private, but as HTTP_Request doesn't inherit from HTTP_Response (which wouldn't make sense) it would be wrong in both cases. While this is one of the things you usually don't note if they use the public API of the package, it does effect "userland" because: a) At least one example does this too (HTTP_Request example download-progress.php, class HTTP_Request_DownloadListener, method update, third line) - so this violation is also "documented" for end-users b) If you need to access private stuff, it shows that the API is flawed - obviously you DO need to access this information from outside. An accessor method would be appropriate there.

Well, since I'm the only active maintainer of the package in question, I should probably respond. I am fully aware of these problems with HTTP_Request. Most of them are results of poor design decisions, though, and cannot be fixed without major refactoring which will desecrate Ye Sacred Backwards Compatibility: instead of having a bunch of getResponse*() methods and an instance of HTTP_Response in private variable, HTTP_Request should instead return an instance of HTTP_Response from its sendRequest() method and HTTP_Response should have accessors for all the parts of the response. An accessor for _url property, though, can be added without any BC problems. I'll consider it for the next minor release. I thought about making a HTTP_Request2, but there is a problem: it makes little sense now to make it PHP4-compatible (as you'll need PHP5-only HTTP_Request3 quite soon) and it makes even less sense to make it PHP5-only, as the package depends on Net_Socket and Net_URL. Therefore, if you want a clearer HTTP_Request, you should lobby for PHP5-only versions of said packages.