[PEPr] Comment on Internationalization::Arabic

First off, this class is insanely comprehensive. I totally see why this is the standard out there. Really well done. I have some (rather random) suggestions, let me/us know what you think. - general suggestions - fix file structure - e.g. I18N_Arabic, should live in I18N/Arabic.php - set-methods could return $this to build a fluent interface - i'd clean-up the naming, so you always have obvious get*() and set*() functions - always avoid private (use protected instead) - avoid require() inside methods - document all classes and methods (run PHP_CodeSniffer) - always implement an "empty" __construct() - this is a question/suggestion: You define a lot of arrays within methods, which make methods really long, what about pushing this data in an external file (role="data" in package.xml) and keeping it there? I don't know how often those are updated, or how extentible they have to be. - autoload - avoid the global variable - (IMHO) don't make it optional, just 'require' it and use it - if it has to be optional, push the configurable variable inside the class - use spl_register_autoload() - with the file structure being fixed, autoload is as simple as: <?php // ... public static autoload($className) { return include str_replace('_', '/', $className) . '.php'; } - error handler - make that switch/flag a configurable variable inside the class (I18N_Arabic::$useErrorHandler) - make the function a public static and wrap it into the class (might conflict otherwise) I looked in some classes randomly, this is what I "found": - __call - document with @method tag - highlightText() - wouldn't hurt to have a default setting for $style - ... and a set (setStyle()) - is hardcoding 'windows-1256' a good idea? - I18N_Arabic_Unicode.constants.php - wrap constants into class constants or class variables (to avoid namespace clashes) - I18N_Arabic_Date::date() - use static variables so they don't get "build" each time it's being used ($hjTxtMonth) - always initialize variables (e.g. $hjTxtMonth) - I18N_Arabic_Transliteration.class.php - fix file name (I18N/Arabic/Transliteration.php) - document your arrays -- wondering if this could improve code readability - what's the difference to I18N_Arabic_Transliteration.php? - please add the package.xml to the repository -- http://pear.php.net/pepr/pepr-proposal-show.php?id=617