Christian Weiske [2009-11-04 08:44 UTC] - You should follow the PEAR package layout: Classes in Arabic/ need have the Class name Arabic_$class.
- I doubt that Arabic is the correct prefix for the package, maybe I18N_Arabic is better.
David Coallier [2009-11-04 12:22 UTC] - If I were you I would read: http://pear.php.net/manual/en/standards.php and to make sure your code complies to the coding standards you can use PHP_CodeSniffer which will help you identify parts of the code that do not comply to PEAR coding standards
- I do think I18N_Arabic could be a better name as well
- What you could do is put your code on github.com and from there we will be able to fork and give you tips (Seeing it online instead of downloading will greatly improve the amount of comments you'll receive)
shady massalha [2009-11-04 14:43 UTC] I already use this package in all my projects. As non-Arabic speakers you may not feel how much relief it caused to us to have this Arabic class. A lot of solutions was made to help Arabic programmers but no one is even close to this one.
Now, I use it in Adobe Flash, Adobe PhotoShop and all softwares i have to write Arabic with embedded fonts and antialias.
See some samples where all made by this package:
http://layan.us/write-arabic-in-flash/ You may see it as simple sample, but to us its WOW.. finally we can write Arabic in flash this way.
http://layan.us/go/arabic_captcha/ Simple Captcha but with arabic letters.
And again in http://muslim-names.us Where i write Arabic Names using GD2 library... And in the same website http://muslim-names.us/%D8%AA%D9%81%D8%B3%D9%8A%D8%B1_%D8%A7%D9%84%D8%A3%D8%AD%D8%B1%D9%81_%D9%85%D9%86_%D8%A7_%D8%A7%D9%84%D9%89_%D8%AE/ Where all these clouds generated in Flash but the text was generated by PHP Arabic Class.
Khaled ilshama`a made the best CLASS to the Arab world. you must approve his solution.
Thanks in advanced.
Ken Guest [2009-11-04 15:10 UTC] Would it seem realistic to refactor portions of this into established packages? for example the spelling of numbers could be refactored into the Numbers_Words package.
Impressive work - though I must admit I18N_Arabic would be a better name for the package. It would also be a good idea to provide unit tests.
Mohamed Waheed [2009-11-04 17:09 UTC] - I see that this package solved many problems in our Arabic projects, it almost have all what we need to deal with Arabic texts, words, date and time very efficiently.
- Given that the programmer uses these features very often, I think it's very reasonable to gather them in one package, under a short and identical prefix like 'Arabic_'.
Philippe Jausions [2009-11-04 17:44 UTC] I am with Ken on this one. I'd be a proponent for refactoring some of the code into existing packages where the functionality fit, or at least making several packages because not all features are I18N per se. Numbers, Date, Text_LanguageDetect, etc...
David Coallier [2009-11-04 18:21 UTC] I am definitely for refactoring, in fact I will not vote +1 on this if it isn't. It is a great package but it has to comply and refactoring is not only for the sake of refactoring.
@Mohamed The idea behind making the code more decoupled is because it is easier to distribute (say you want to use only "x" part of the arabic package, you don't need the whole package, only part "x") and obviously the quality of each package can be maintained in a much more controlled way. By having a variety of independent packages, you can insure better unit testing of each package and that is a great asset.
What I would suggest is that you try to identify all the parts that are independent to one another and see which is the real core, and how all the pieces are stuck together.
This means you'll have one main package which will depend on all the smaller packages however the smaller packages will NOT depend on the bigger ones thus making them independent.
If you need help on how to refactor your code I would highly suggest sending an email to the pear-dev mailing list to request comments on how to do it.
On another note, the name "Arabic" does not fit a category name. It is Arabic in the Internationalization category (i18n). So something like I18N_Arabic makes much more sense and I'd be surprised to see "Arabic_*" accepted :)
Khaled Al-Shamaa [2009-11-05 06:58 UTC] I agree with you that I18N_Arabic will be better name for this package, thanks for your advice, I will do it.
Thanks Christian for your advice in naming sub classes using PEAR standard, I think it should be I18N_Arabic_$class
Dear David, source code already hosted on sourceforge.net, sorry because I did not mention that before, you can find it here: http://sourceforge.net/projects/ar-php/
Well, I wish to shift it into github.com, but I don't know an easy way to import my CVS repository and keep tracking history … himmm, it seems that we have a lot of changes in file and directory names (which can’t be tracking by CVS), so I believe it is a good time to get a new start point using better technology like Git instead of CVS.
Dear David, thanks for your advices to use PHP_CodeSniffer, I will do scan all of my files using that tool and fix all reported issues, so … I have to do my homework then back to you :o)
Dear Shady and Mohamed, thanks for your support, but at the end of the day, standards are standards, and one of the PEAR success cause is their high level standards.
Khaled Al-Shamaa [2009-11-05 07:30 UTC] Dear Ken, Philippe and David,
Related to the refactoring issue, I feel that split this package into smaller packages will make it harder to maintain and implement, please pear in your mind that most of the package functionality share the core of transparent charter set converter (actually roughly of Arabic websites used UTF-8 while other half still used Windows-1256 with a small share for ISO -8859-6).
On the other hand, we already implement semi-factory pattern, so logical modules are separated in different sub classes in different files and loaded dynamically on request, this structure keep code maintainable.
I like to think about it as Swiss-army knife for Arabic website developers where they can find most of their day to day needs (related to Arabic language), so pack it in one PEAR package will make implementation and deployment much easier and needs less memory footprint.
In light of those facts, do you still believe that it is better to have several small packages? I am open to any suggestion that will improve this piece of work.
Till Klampaeckel [2009-11-08 19:10 UTC] 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
Khaled Al-Shamaa [2009-11-09 07:03 UTC] Thanks Till for your time, I will response every piece in your valuable comments and do my homework
Khaled Al-Shamaa [2009-11-09 13:22 UTC] Dear PEAR members,
We know it is challenging task! We develop this library since Feb 2006 as standalone library, and we believe that shifting it into PEAR standard package will not be a trivial task, so please be patient and positive, and keep feed us your comments and hints to enable us get benefit from your experience in this process, and be sure that we will pay all needed efforts to accomplish this task whatever time needed.
At the end of the day, we believe that enable this library as standard PEAR package will make it one of the best resources available to the PHP developers in Arab countries.
Best regards,
Khaled
Arash Hemmat [2011-12-26 11:48 UTC] Hi Khaled,
you did hell of a job! It is really impressive, it is much more than just a single Arabic i18n library, i can understand how important these classes are to Arab users.
I have some suggestions for you:
1. refactor the date conversion class to a new Pear package, it is a much needed package for future projects on Pear. why? you are aware that many countries in middle east use calendar systems other than Hijri, however some of their holidays are based on the religional events based on Hijri calendar, if you take a look at Pear Date Time category you won't find Holiday packages for countries like Turkey,UAE,Iran,... because most of their holidays are calculated based on Hijri while they use other calendars, if there was a package converting date to Hijri making such packages would be possible.
2. you may be also interested in supporting PHP IntlDateFormatter class which supports Hijri calendar.
Daniel O'Connor [2012-01-01 07:27 UTC] @Arash - can we fork some of this into smaller pear packages? IE: Date_Holidays & similar?
Daniel O'Connor [2012-01-01 07:27 UTC] @Khaled - perhaps a good way forward for you is a pear channel. Have you seen Pirium?
Arash Hemmat [2012-01-02 08:52 UTC] I think the Hijri date convert class could be a seperate package and in the future it could be used as a requirement in other packages which could be really helpful. About the rest i think the current package proposed by Khaled is fine if he can make changes to meet Pear standards.
Khaled Al-Shamaa [2012-03-04 10:39 UTC] I submit this post to reactivate this discussion after we released version 3.0.0 of this library month ago, it is now more than 6 years old project, and the main goal was always to have one stop shop solution enable developers to serve professional search, present and process Arabic contents in PHP language.
In the latest version we implement so many advices presented here in this thread of discussions, including error free phpcs reporting and new directory/file/class naming and structure, but still we have some issues in our to-do list, and I’ll back to you once I feel that I fit your requirements.
To be honest, the main objective of this project is to support Arabic language in PHP, so refactoring portions of this work into established PEAR packages will complicate our mission!
Dear Arash and Daniel, please be sure that I will not hesitate to provide any required help related to this issue. On the other hand, where I can get a quick short answers and advices during refining process to make this package qualified to be listed in PEAR repository?
Khaled Al-Shamaa [2012-03-04 12:35 UTC] Is there a way that I can use to update the proposal description and update existing links as a response to the PEAR community feedbacks when status is "Proposed"?
For more information about this project, please visit its homepage:
http://www.ar-php.org/en_index-php-arabic.html
|
