Re: The RPCBlogging package

On Mar 18, 2006, at 11:36 AM, Anant Narayanan wrote:

Hello All, I just proposed the Services_RPCBlogging package and invite comments on it. Some notes: Sorry for replying here, but I left my PEAR login on another system.

- You're using PHP 5, but RPCBlogging_Post is littered with get/set methods. You should use PHP 5's __get() and __set() - You seem to have driver-specific code in RPCBlogging_Post; switch (RPCBlogging::$driver) in __construct(). The Post class should be naïve, and this should be handled by the drivers. - This check - if (!($content instanceof RPCBlogging_Post) - can be replaced with a type hint in the function prototype. I see it in the metaWebLog driver; if it's present in the other drivers, it could be fixed there as well. - Is there any reason you're using the XML_RPC PEAR package instead of PHP 5's native XML-RPC extension? - You need to get your directory structure in line with PEAR standards. Services/RPCBlogging/RPCBlogging_Blogger.php should be something more like: Services/RPCBlogging/Driver/Blogger.php. - RPCBlogger::factory() needs work. It should also be naïve, and just try to load a class/file generated from the driver you ask for. With your if/else setup, you'll have to modify it every time you add a driver. It should be something like: public static function &factory($driver, $options = array()) {

    $class = 'RPCBlogger_Driver_' . $driver . '.php';
    $file = 'RPCBlogger/Driver/' . $driver . '.php';
    incluide_once $file;
    if (!is_class($file)) {
        throw new Execption('Driver not found: ' . $driver);
    }
    $inst = new $driver($options);
    return $inst;

} - You should move the functions needed by the drivers out of RPCBlogger and into RPCBlogger_Extended; RPCBlogger_Extended shouldn't extend RPCBlogger. Extended could be renamed to RPCBlogger_Driver, and form the base class for the other drivers, since that's basically what it already does. - Class names need to have the name of the package, i.e. Services_RPCBlogger_Extended. - I'm not sold on the name. Perhaps Services_Blog ?