Comments for "Services_TwitPic"

» Submit Your Comment
Comments are only accepted during the "Proposal" phase. This proposal is currently in the "Finished" phase.
» Comments

  • Chuck Burgess  [2008-08-20 16:57 UTC]

    Might consider using give-Nazg-a-clue-what-it's-for constants for those "200" and "500" codes used in Services_TwitPic::sendRequest() (line 190) and the unit tests. I'd guess both constants would be declared in the TwitPic class itself.

    Maybe add a note to sendRequest's docblock indicating that usage of @simplexml_load_string, in case a user debugging some failure needs to know that such error suppresion is occurring.

    Very nice touch including a Mock object for the request object...

    Unless the CS requires it, you can drop the @access, @abstract, and @static tags... phpDocumentor doesn't need them, or indeed even use them at all, in PHP5 code. I think those tags only really existed for making your intent explicit when PHP4 code had no way to actually do it.

    Constructors don't usually use the @return tag, but theoretically such @return tags should return an instance of the class rather than "void".

    In some cases, the @return tags where an object is being returned could actually directly show that object class as the datatype:
    * @return object Instance of TwitPic_Request_Common
    * @return TwitPic_Request_Common the returned object
    As long as phpDocumentor is already documenting that returned class in the same documentation run, it should make that datatype into a link to the class's doc. I don't think this would work for:
    * @return object Instance of SimpleXMLElement
    * @return SimpleXMLElement a simpleXml object
    (unless "SimpleXMLElement" itself is already a "native" PHP datatype.)

    Since you're already doing a fine job explicitly pointing to related methods via the @see tag, you might also consider this change:
    * @return mixed Results of sendRequest()
    * @return mixed Results of {@link sendRequest()}
    so that the sendRequest() will be a link to that method in the generated docs.

    I fear I might be scrutinizing from an overly-phpdoc'd perspective ;)

    I do like the logical function layout and the small methods... very clear code to read and logical API to follow.

  • Bill Shupp  [2008-08-21 06:11 UTC]

    Chuck,

    Thanks for the detailed response. I implemented all of your suggestions, and they are available in Services_TwitPic 0.0.2 here:

    http://servicestwitpic.googlecode.com/files/Services_TwitPic-0.0.2.tgz

    All PHPDocumentor errors (in errors.html) are now gone. :)

    Cheers,

    Bill