Re: Proposal: Services_ExchangeRates

Hi, I like this code. A few notes: /**

    * Sets the length (in seconds) to cache the exchange rate data. This information
    * is updated daily.
    * @var int Cache length (default is 1 hour)
    */
    var $_cacheLenghtRates = 60 * 60 * 1; // sec/min * min/hr * hrs

$_cacheLenghtRates

           ^^

retrieveData() will give PHP warnings if the driver is missing, perhaps you could encapsulate the include_once() call like so:

    function retrieveData($source, $cacheLenth) {
        $classname = "Services_Exchange_$source";
        if (!class_exists($classname)) {
            include_once("Services/Exchange/$source.php");
            return PEAR::raiseError("No driver exists for the source ${source}... aborting.", true);
        }
        $class = new $classname($cacheLength);

        return $class->retrieve();
    }

You'll still get the PHP warning, but only for typos when a warning is appropriate. I'd like to see error codes used before the package is accepted, it will give the package a wider audience. I'd also recommend using a system similar to Cache_Lite - include PEAR.php if you have an error, and otherwise, don't. Some users will like the speed difference (just a recommendation). A more serious problem, the example displays: Parse error: parse error, expecting ','' or ';'' in /home/exclupen/public_html/misc/pear/Services/ExchangeRates.php on line 64 Fatal error: Cannot instantiate non-existent class: services_exchangerates in /home/exclupen/public_html/misc/pear/Services/ExchangeRates/docs/example.php on line 37 :) Greg Marshall Roch wrote:

Marshall Roch wrote:

I've incremented the version number to 0.4, with the following changes:

The URL is still the same, but I'll post it again so you don't have to dig for it: http://www.exclupen.com/misc/pear/Services/ http://www.exclupen.com/misc/pear/Services_ExchangeRates-0.4.tgz Sorry for forgetting to include these two times in a row! :) -- Marshall Roch