Vote Details for "AsteriskManager" by till
| » Details |
|---|
|
| » Comment |
|
* create your own exception class (extending PEAR_Exception) * use a code when you throw an exception, e.g. implement codes through class constants and e.g. use self::ERR_NO_SOCKET. this allows people to check what kind of exception they received. * if i supply auto_connect = false, it would still call connect() (from __construct()) * connect() doesn't return anything (doc problem) * wrap your "if (!$this->_socket)" into a method and also check if it's a resource (not just if it's not false) * instead of fputs(), you should use fwrite() (imho) * fputs/fwrite can return false, shouldn't that be checked? * stream_get_contents can also return false (Maybe wrap those calls into their own method and throw an exception if it doesn't work for some reason.) * simplify your if/else (most time the else is not necessary) * unless PEAR_Exception implements toString(), you will have to do echo $e->getMessage() in your example Last but not least, I'd like to see more of the API supported (this can be done on the way to 1.0-stable), regardless - what's your own plan/roadmap for this? One last suggestion/question - I wonder if it would make sense to subclass e.g. create Net_AsteriskManager_Queue, Net_AsteriskManager_Sip, *_Zap, *_Monitor, *_Mailbox, etc.. This would maybe help organize the feature-set, otherwise you end up with a couple dozen methods in Net_AsteriskManager. |