[PEPr] Comment on Networking::Net_DNS2
| From: | Michael Gauthier | Date: | Thu, 19 Aug 2010 14:23:54 +0000 |
| Subject: | [PEPr] Comment on Networking::Net_DNS2 | ||
| References: | 1 | Groups: | php.pear.dev |
| Request: | Send a blank email to pear-dev+get-53691@lists.php.net to get a copy of this message | ||
Great start. There's a bit of work left to do to get this proposal ready
but it's showing promise already.
1.) Use of the global __autoload() should not be part of this package. PEAR
packages should use something like "require_once 'Net/DNS/Header.php'"
2.) Run phpcs on the code to find many coding standards errors. Spaces
should be used instead of tabs and most methods and properties are missing
documentation.
3.) Consider the MIT, Apache2 or BSD licenses.
4.) Consider the magic __toString() method instead of string() for
debugging.
5.) Use class constants instead of @define constants.
6.) Code that was adapted from Net::DNS PERL code needs to respect the same
license and should state the license in the file header.
7.) empty destructors are not needed
8.) Does the 'sockets' extension provide any benefits over the 'steams'
extension. Is it worth having two Socket implementations if streams works
just as well and is always enabled?
9.) no need to check if ($bool === true). You can just do if ($bool).
10.) If having multiple socket implementations makes sense, consider adding
a method of dependency injection where you can set the socket object on a
Resolver. This will make unit testing easier as you can create a mock
socket object.
11.) Same for the Lookups class.
12.) Unit tests!
--
http://pear.php.net/pepr/pepr-proposal-show.php?id=643