Comments for "Net_DNS2"

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

  • Daniel O'Connor  [2010-08-19 02:59 UTC]

    Don't forget unit tests :)

  • Michael Gauthier  [2010-08-19 14:23 UTC]

    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!