Matthew Weier O'Phinney [2005-12-28 20:47 UTC] 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;
}
bertrand Gugger [2005-12-31 08:08 UTC] It's a clean integration of several pear packages.
* package's name: should be Services_OpenSearch as I guess it should be in the Services category
* CS for methods, even if very short methods should not be packed on one line, but rather have their block starting on next line as:
function getAdultContent()
{
return $this->_getDescription('AdultContent');
}
* Shouldn't private methods names start with an underscore ?
* It should be possible to change the url and not need to instanciate a new one for it, perhaps that could be combined to solve Matthew's question.
* That would be nice to be able to give the pager's options directly by instanciation as an options array
* I would check the descrition returned in fetchDescription($url) has effectively the '_got' element as it's a key for next extractions and not rely on the data correctness
* Not sure if some cache mechanism could not save some requests here ...
* In setupQuery(), your use of str_replace() is incorrect, both search and replace are simple arrays, the hash for replace has no meaning, only element's order is important.
HIROSE Masaaki [2006-01-04 15:13 UTC] Thanks for great advise.
I accept all comment and try to modify Services_OpenSearch. A little time please.
|
