[PEPr] Comment on System::System_Serial

Ian Eure (http://pear.php.net/user/ieure) has commented on the proposal for System::System_Serial. Comment: Some additional issues: 1. Why do you have a System_Serial_Factory class, instead of a System_Serial class with a factory method? 2. What is the point of the switch in System_Serial_Factory::NewConnection? You seem to drop-through to an if statement, so the switch appears to just be bloat. 3. Why is there a function which only returns the PHP_OS constant? Why not use it directly? 4. More CS stuff; FALSE should be changed to false. 5. I think there may be path-related security implications to calling "mode" in System_Serial_Win32COMconnection::init() without a full path. Is "mode" a builtin function, or an external command? If it's external, it should have the full path added. 6. Any reason why System_Command couldn't be used to execute these commands? I believe it handles the path-related issues I raised in point #5. 7. I think the class names are misleading. COM connetions could work on 64-bit Windows, not just Win32. And many *NIX variants use /dev/(whatever) syntax for serial ports. 8. I'm not convinced that seperate classes are necessary, in any case, since it's only initialization that's platform-specific. Why not have a single class with e.g. _unixInit() and _winInit()? 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