RE: [PEPr] Comment on Networking::Net_DNS2

Comments in-line Mike >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'" Yup- make sense- >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. Yup- I still need to go through and add docs to the tops of the function calls; the spaces vs tabs- I can't stand using spaces when I'm working on an app, so I had planned on simply converting them to spaces before the release. ;) >3.) Consider the MIT, Apache2 or BSD licenses. I generally use a BSD license for my code- I thought I had read somewhere that using the PHP license was preferable. >4.) Consider the magic __toString() method instead of string() for >debugging. I'm using that in some places; for example, the Packet object uses it, and iterates through all its member objects to produce a full packet output. So you can just echo a $packet; >5.) Use class constants instead of @define constants. Yup- I had planned on moving them, I just was not 100% sure yet where they were going to live; >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. Yup- there's only a few RR's that I borrowed code directly from Net::DNS. >7.) empty destructors are not needed I just haven't used them yet ;) >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? The sockets extension is noticeably faster; so if it's there, it's worth using considering the speed advantage. >9.) no need to check if ($bool === true). You can just do if ($bool). It's a old habit; I find it much more clear what *exactly* I'm testing against when you specify the value; especially since PHP seems to have a long running in-consistency with what it considers to be "true" and "false". For example, in a case like: socket_write() where it returns the number of bytes or FALSE on error; there's no way to tell the difference between 0 bytes and an error, without ===. There's lots of these cases, and I prefer to be 100% clear what it is I'm testing, vs letting PHP decide. Also, == vs ===, in testing- I've found === slightly faster than ==, it can also catch weird cases when the data is not the type expected. >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. Yup- that's a good idea- I could make a dummy socket class that produced packet data as expected, that could be used to test the parsing and other logic. >12.) Unit tests! Yup- they're coming,