Re: Package proposal Science_Weather

Hi Alexander, This is high quality code, I honestly love it :). I'm pretty sure you need not extend PEAR - the destructor registration will slow down your application, which will make it unattractive to sites (increased load time = bad). You can still define a throwError() method that will pass in the Science_Weather_Error classname as the last parameter to PEAR::raiseError(). In addition, I'd recommend following the model of Cache_Lite, and only include the PEAR.php file if an error condition actually occurs. I'd like to see just a bit more verbose documentation before giving a rating. You can see why this is necessary by generating phpDocumentor docs from the source. As it stands, they would be pretty much useless because they don't have quite enough detail. Specific examples:

    /**
    * Sets the neccessary account-information for weather.com
    *
    * @param    string      $partnerID
    * @param    string      $licenseKey
    * @access   public
    */
    function setAccountData($partnerID, $licenseKey)

Could you provide a link to the page on weather.com that relates specifically to this data, or at least describe in the @param tags what they are (something like "partnerID is the username assigned by weather.com" or "partnerID is your email address" - or whatever it is :)

    /**
    * Changes the representation of the units (standard/metric)
    *
    * @param    string      $unitsFormat
    * @access   private
    */
    function setUnitsFormat($unitsFormat)

Could you describe that $unitsFormat is a 1 character string, either 's' or 'm'? (I'm assuming here - the docs should correct me if I'm wrong) It's not yet a standard, but you might want to specify which error codes are thrown with @throws as in: @throws Science_Weather_Error::SCIENCE_WEATHER_ERROR_WRONG_SERVER_DATA This gives a little more information on which errors need to be handled when calling a method. (If you decide against this, it won't affect my vote) The @return tag for function getUnits($id = "", $unitsFormat = "") should be @return array, not @return mixed. In addition, please document each of the possible returns, since it's pretty clear that there is a limited set of returns, and it's a little tricky to figure it out just by reading the source (plus phpDocumentor-generated docs will be more readable). I have the same comment for function getLocation($id = "") - the @return should be array(), and should be documented in as much detail as succinctly possible. Same for getWeather(). I think you get the point :). If you want your package to be approved any sooner, you should provide a link to a .phps as Arnaud requested, not everyone will have the time to download the package and extract the source like I did. Your code is extremely clear, and follows CS to the T. I will be very enthusiastic when the documentation matches this level of excellence :). Greg Alexander Wirtz wrote:

Howdy, I wrote a small class which retreives XML data from weather.com. As there's no _working_ code out there, doing that task, I'd thought that it might be a good addition to PEAR. It is available under http://www.pc4p.net/downloads/Science_Weather-1.0.tgz The class is already packaged and ready to install... Feel free to contact me for further suggestions or rants (not). Regards, Alexander