[PEPr] Comment on Web Services::OpenSearch
| From: | Matthew Weier O'Phinney | Date: | Wed, 28 Dec 2005 20:47:24 +0000 |
| Subject: | [PEPr] Comment on Web Services::OpenSearch | ||
| References: | 1 | Groups: | php.pear.dev |
| Request: | Send a blank email to pear-dev+get-40847@lists.php.net to get a copy of this message | ||
Matthew Weier O'Phinney (http://pear.php.net/user/weierophinney) has commented on the proposal
for Web Services::OpenSearch.
Comment:
For forward compatibility with PHP >= 5, I'd suggest using a factory for
instantiation; constructors in PHP >=5 cannot return an object instance of
another class. So, the following will cause errors in PHP5:
function Services_OpenSearch($url = null)
{
if (! is_null($url)) {
$this->_descriptionUrl = $url;
} else {
return PEAR::raiseError('OpenSearch: missing URL.');
}
// ...
}
Do a factory or singleton instead.
Also, it would be nice if your constructor/factory/singleton would allow
setting the various parameters of the $pager_param array. You could do
this with an optional second argument:
var $pager_defaults = array(
'count' => 10,
'startIndex' => 1,
'startPage' => 1,
'totalResults' => -1,
'itemsPerPage' => -1
);
function construct($url = null, $pager_params = array())
{
// check for url
// ...
// Set pager params
if (!empty($pager_params)) {
$this->pager_param = $pager_params;
}
// Get defaults for unset pager parameters
foreach ($this->pager_defaults as $key => $default) {
if (empty($this->pager_param[$key])) {
$this->pager_param[$key] = $default;
}
}
}
Finally, please follow CS conventions, particularly the 'one true brace'
convention. The following:
function getCount() { return $this->pager_param['count'] = $n; }
while succinct and valid PHP, breaks from PEAR CS, which would have you
format that as:
function getCount()
{
return $this->pager_param['count'] = $n;
}
Proposal information:
http://pear.php.net/pepr/pepr-proposal-show.php?id=336
--
Sent by PEPr, the automatic proposal system at http://pear.php.net