Matthew Weier O'Phinney [2006-05-31 17:10 UTC] * s/syncronize/synchronize/
* Don't use is_object() to check for errors; use PEAR::isError()
* How will you deal with differences in timezones between a web server and the local filesystem?
* Use require_once instead of include_once. In each instance that you use it, it would be better to raise a fatal error on loading the class file than to later get it when the class isn't found. (Die early)
* In HTTP_Sync_Download::raiseError, use __CLASS__ instead of the string 'HTTP::Sync::Download'.
Philippe Jausions [2006-05-31 18:09 UTC] IMO it is not a good idea to extends HTTP_Request. Instead you should use an internal instance of HTTP_Request. The reason behind this is that when HTTP_Request gets support for other backend (i.e. cURL, stream...) you package wouldn't benefit from the upgrade.
The way you have it, the get() method could fail returning any content, even if a local outdated file is present. So, I'm not sure if the get() belongs in there, or maybe it's just misnamed. I would rename "synchronize()" to "download()"; and "get()" into "synchronize()", and leave the file_get_contents() to userland (which is BTW not always available.)
Ildar Shaimordanov [2006-06-02 04:49 UTC] re: for Matthew
Thanks for your remarks. I corrected my code accordingly them.
Some words about "differences in timezones"
I think so
The local server as receiving system have to know about timezone of the remote system. So that, the local system must correlate local timezone to remote one.
E.g.:
if the remote system timezone is GMT,
and the local timezone is MSK,
and the remote system's lastModified time is 03:40,
then local lastModified is remote lastModified + (int)date('Z')
Ildar Shaimordanov [2006-06-02 06:07 UTC] re: for Philippe
I'll think about Your comments
As I understand You recommend the following
class HTTP_Sync
{
function HTTP_Sync($target, $source, $params)
{
$this->_request =& new HTTP_Request($source, $params);
...
}
Ildar Shaimordanov [2006-06-08 12:18 UTC] Is it necessary to add value $target === null, meant for skipping download file and for returning file contents only&
|
