[PEPr] Comment on File System::File_Manager_System

Philippe Jausions (http://pear.php.net/user/jausions) has commented on the proposal for File System::File_Manager_System. Comment: Oh, boy... Let's start with the educational stuff: - fms_if() try ?: ternary operator instead. - Use PHP_CodeSniffer package to fix all the PEAR Coding Standard issues. - Do not pollute the global namespace with functions, use static method instead. - Do not define global constants for options, that prevents from changing the behavior of the class from one instance to the other. - Don't use eregi* functions, use simple string or preg_* functions. - Leverage existing packages if possible, i.e. MIME_Type, File for instance. - Don't EVER use eval() (at least not until you have a LOT more experience). Learn a bit more about variable variable and variable function first. - Use exceptions instead of always returning FALSE on error (FALSE doesn't provide much information, whereas an exception can) - Use __call() for your cmd(). Why a use an error-prone string parsing-based approach when the rest is OOP? - Default a parameter to NULL instead of FALSE Things like: if (SHOW_SECRET_FILES === FALSE && $result{0} != '.') { $files_stats++; } elseif (SHOW_SECRET_FILES === TRUE) { $files_stats++; } are much simpler written: if (SHOW_SECRET_FILES || $result{0} != '.') { ++$files_stats; } Now, have you looked at SPL? Seriously, as-is, the package is not PEAR-quality. Rework it around SPL, learn the PEAR coding standard, avoid pitfalls like eval(). I'd also suggest to look at existing PEAR packages and how you could contribute to them for some convenience methods. Proposal information: http://pear.php.net/pepr/pepr-proposal-show.php?id=530 -- Sent by PEPr, the automatic proposal system at http://pear.php.net