[PEPr] Comment on Networking::Net_CDDB

Justin Patrin (http://pear.php.net/user/justinpatrin) has commented on the proposal for Networking::Net_CDDB. Comment: There seem to be a lot of constants.... I suggest for the drivers just to use a straight string instead of a constant. if ($cdreader == NET_CDDB_READER_USERDEFINED) { ; } else { huh? Just use != connect() and disconnect() aren't returning anything if they aren't connecting ir disconnecting. Store strpos($this->_buffer, "\n") instead of re-calling it 3 times. Something like: if (false !== ($pos = strpos($this->_buffer, "\n"))) { Don't use () with return. Always use {} with blocks, even if the block is one statement. (such as with the foreach in calculateDiscId() and elsewhere.) use Net_Socket instead of using sockets directly. submitDisc() should be returning a PEAR_Error on error. $err isn't used at all... The many parameters to Net_CDDB_Disc's constructor seems to be a bit much. Why not just use an associative array? You don't need a seperate factory class. Put those functions in the main class. Don't use dirname(__FILE__)! Include as from the include_path. Always. connect() ought to return a PEAR_Error if it fails. (PEAR::raiseError()) use urlencode() or rawurlencode() instead of a direct str_replace(). Proposal information: http://pear.php.net/pepr/pepr-proposal-show.php?id=320 -- Sent by PEPr, the automatic proposal system at http://pear.php.net