Vote Details for "Services_GeoNames" by gauthierm

» Details
  • Voter: Michael Gauthier 
  • Vote: +1 (not conditional)
  • Reviews: Cursory source review
» Comment
Looks good to me.

I'm not certain it's a good idea to make your HTTP_Request2 object a public property. In my opinion, it would be better served as a protected property with a public setter method.

Also, you should clone the request (use it like a template) rather than reusing. See the discussion on http://pear.php.net/bugs/bug.php?id=15072 for details. This is implemented in Services_Amazon_SQS as well.

More specific exception types would be useful, as would documenting under which conditions exceptions are thrown in the @throws sections.

I see you've already updated the code based on other people's suggestions, so that's all I've got :) I like how you're including the response object in exceptions caused by HTTP. I'll have to add that to my packages.