[PEPr] Comment on System::System_Serial

Hello, Thanks for your helpful critique. The code in System_Serial has been a part of a bigger program and inherits it's oddities from that context. I've changes to the code as suggested by Stephan Schmidt, Ian Eure and Martin Jansen. The things left unchanged are: - PHP 5 is required indeed by few features aside from variable visibility in the code, but singleton for one would not work without $_instance being static. - I'd wish the System_Serial package not to be dependend on any other packages, so pairing it with System_Command is something I'd like to avoid. I solved the possible security issue in calling "mode" in Windows by inserting the full path to the command. - What with the getLine() and getArray() function names, I've kept them as they were on the logic that as those functions use fgets to read the port it's to me clearer to call them get...() and also if I sometime in the future write other functions using fread (or similar), the appropriate names would already be taken. >>> "Martin Jansen" <mj@php.net> 24.09.04 11:58 >>> Martin Jansen (http://pear.php.net/user/mj) has commented on the proposal for System::System_Serial. Comment: In addition to the other comments a bit more: 1) PEAR has the so called "one-class-per-file" convention, which means that multiple classes should end up in multiple files. 2) Why don't you simply use return PHP_OS; in _determineOS()? 3) I'd suggest to rename getLine() to readLine() and getArray() to readLines(). 4) I'm by no means an expert in serial communication, but shouldn't there be a way to change the value of $badCommandResponse in getArray()? (There may be systems that use a different protocol dialect.) 5) I'm missing documentation for the package. Are you going to write some? Generally this looks like a useful addition for PEAR. Proposal information: http://pear.php.net/pepr/pepr-proposal-show.php?id=149 -- Sent by PEPr, the automatic proposal system at http://pear.php.net -- PEAR Development Mailing List (http://pear.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php