[PEPr] Comment on Console::Console_CommandLine

David Jean Louis (http://pear.php.net/user/izi) has commented on the proposal for Console::Console_CommandLine. Comment: Hello Travis, wow, very nice code review ! I'll try to answer the best I can to your remarks: >> The biggest thing that jumps out at me is the name. >> Is Console_CommandLine the best name for this? fair enough, in fact the library was initially called Console_OptionParser (as the main class of optparse python module) but I though it was to restrictive because Console_CommandLine handles options but also arguments and sub-commands - in other words a whole "command line". OptArgParser doesn't sound very good to me, maybe CommandLineParser would be more appropriate ? Need thinking... >> I would move to an Outputter class that could be injected at instantiation. very good idea, I'll implement it asap :) >> In the Renderer class I would remove the render prefix... another good idea, +1 >> Why are there so many protected methods within the Renderer class? you're right they should be public, I'll fix this, thanks. For the double foreach, it's because we have to calculate the max length of text before calling columnWrap() method, but maybe it could be done better, I'll look at this. >> [...] the number of public properties is too high for my tastes. >> I would move them to protected/private and add accessors via __get()/__set() hmm, not sure about the magic methods idea, I'll think about it and keep you informed >> I would also move dispatchAction() [...] yep, good idea ! the possibility of defining custom types is definitively cool. >> I'm not sure I see the reasoning behind the various static properties [...] I put these static properties to keep messages and regexes in the same place, rather than having to search them in the code each time a change has to be done, static because they are the same in all instances, I'm opened to suggestions if it could be done in a more elegant way :) >> why are there trigger_error() calls in validate instead of throwing an Exception? Because I think that programming errors should just trigger_error and user errors (like bad usage of the program) should raise Exceptions, so that the programmer can display the error message to users with getMessage(), you think it would better to treat all errors as exceptions (with different execptions depending on the type of error, ala python) ? >> This is an excellent base to work from. And this was a very useful review, thanks ! Proposal information: http://pear.php.net/pepr/pepr-proposal-show.php?id=517 -- Sent by PEPr, the automatic proposal system at http://pear.php.net