[PEPr] Comment on Authentication::NTLMProxy

Thorsten Rinne (http://pear.php.net/user/thorstenr) has commented on the proposal for Authentication::NTLMProxy. Comment: Hi, > sprintf is far slower than any other string creation method. Single quotes > are the preferred method. See: > http://pear.reversefold.com/strings/ > It is especially inappropriate to use sprintf when you're not even doing > anything to the string (such as sprintf("\r\n\r\n")...). if speed is the only problem then I'll be happy. ;-) I remove the unnecessary sprintf(). > This package should use HTTP_Request and/or HTTP_Client. If there is a > problem with keep-alive I doubt it would be too hard to alter HTTP_Client > and Request to support it. They could easily just keep the socket open and > do more sending/receiving. If it's easily, please fix http://pear.php.net/bugs/bug.php?id=4806 first. > I don't really like the names of the "state" functions. I would rather > they were named for what they do such as connect, sendRequest, > readRequest, parseRequest, etc. Okay, I'll do this. > die()/exit should not be called in a package. Why can't the script simply > exit normally? The need of die() is that there's a rea big problem when this situation happened... The exit() calls are needed during the NTLM handshake. > _createNewSocket and _createSocket duplicate code. They don't duplicate code, _createNewSocket() closes an open socket first. > Don't use @ to silence notices and warnings. Write your code to avoid them > instead. I'll add that. > Errors should raise/return a PEAR_Error. okay, I could use it. > The log is a decent I idea, but I suggest you use the Log package instead > as it would allow the developer to log however they want to. I do not want a bloated class. -Thorsten Proposal information: http://pear.php.net/pepr/pepr-proposal-show.php?id=396 -- Sent by PEPr, the automatic proposal system at http://pear.php.net