Michael Gauthier [2009-03-17 13:17 UTC] Great start. I've got a lot of feedback, but don't be discouraged.
- use the PHP Countable interface since you implement a count() method.
- add type documentation for class properties (@var string, etc)
- either use repeating docblock syntax or document all class properties
- use the 3rd person declarative rather than the 2nd person imperative in documentation. "Sets the foo" rather than "Set foo". See http://pear.php.net/manual/en/standards.sample.php
- documentation is not quite ready yet. See the PEAR manual standards section for a guide to writing good API documentation.
- throw exceptions instead of maintaining an error status
- don't suppress errors using the @ operator
- when building the URI, consider using the urlencode/rawurlencode functions, or use the PEAR::Net_URL2 package to do it for you.
- consider using PEAR::HTTP_Request2 instead of file_get_contents(). This will allow mock requests for unit testing and will allow you to handle errors without having to suppress them or write a custom error handler.
- methods need @param and @return documentation. Try running your code through phpcs (http://pear.php.net/package/PHP_CodeSniffer) to see other missing documentation bits.
- having default parameter values that are invalid doesn't make sense. Instead of allowing no parameter and then setting an error condition, just make the parameter required.
- documentation for what array keys are required for the lookup methods is needed.
- variables should use $camelCase rather than $under_scores.
Michael Gauthier [2009-03-17 13:18 UTC] Also, the package should be named Services_WitePages since it is in the PEAR WEb Services category.
Till Klampaeckel [2009-03-19 13:40 UTC] Juan,
Michael gave you great feedback already. Please let us know if you we can shed some light on some of the items and help out, etc.. :-)
Cheers,
Till
Juan Manuel Fernandez Arauz [2009-03-23 18:22 UTC] Hi, thank you for your answers, im very grateful for you support. I will update the package as fast as i can; im really interested in posting this project.
I already change the name of the package to Services_WhitePages.
Kind regards.
Christian Weiske [2009-06-25 08:07 UTC] Is this proposal dead, or are you going to finish it?
Michael Gauthier [2011-01-30 21:57 UTC] Juan, are you still working on this proposal?
|
