[PEPr] Comment on File System::File_Launcher

Consider structuring your package slightly differently: File/ File/Launcher.php That lets you run tests straight out of svn/git without doing: require_once dirname(__FILE__).'/../File/Launcher.php'; Even better - it'll slot right into the unit test framework :) I'd avoid doing work in the constructor (it makes unit testing harder): public function __construct() { $this->nCurrentOS = $this->detectOS(); } I'd consider splitting it into driver type classes if you see more in depth functionality being required. This also would allow people to easily mock out the launch command. class File_Launcher { // Snip: public function getCommand() { return $this->os->getCommand(); } public function detectOS() { foreach ($this->drivers as $driver) { if ($driver->applies()) { $this->os = $driver; return; } } throw new Exception(":(") } } class File_Launcher_Foo implements File_Launcher_Driver { /** What's the command to launch things? */ public function getCommand() {} /** Does this apply to the current operating system? */ public function applies() {} } Final one: static methods make unit testing harder - I know you want to make it trivial to do File_Launcher::launchBackground() as a one liner, but I'd urge against it - $foo->launch(..., true or false) tends to cover it. Overall: I like it -- http://pear.php.net/pepr/pepr-proposal-show.php?id=642