Refactor country metadata into a single function
malaussene - March 19, 2009 - 23:52
| Project: | Phone (CCK) |
| Version: | HEAD |
| Component: | Code |
| Category: | task |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs review |
Description
Hi,
here is a patch cleaning up code duplication by centralizing the declaration of each country metadata (error, label...).
It then simplifies the addition of new countries.
malaussene
| Attachment | Size |
|---|---|
| phone-refactor.patch | 13.58 KB |
#1
I like the idea but there seemed like there were a lot of helper functions. I've slimmed it down to just phone_country_metadata(), phone_countrycode_from_fieldtype() and _phone_field_types(). Also provided documentation for them. I dropped the 'cc' field from the metadata since it just repeated the key's value.
#2
better title.
#3
Further refactor by moving out the country metadata into its own .inc, so the phone module is clean from country specific stuff. Now country can be added/removed easily and without the need to update phone.module.
Each country just need to implement phone_CC_metadata, where CC is the country code.
e.g. for es
function phone_es_metadata() {// These strings are translated using t() on output.
return array(
'label' => 'Spanish Phone Numbers',
'error' => '"%value" is not a valid Spanish phone number<br>Spanish phone numbers should only contains numbers and spaces and be like 999 999 999',
);
}
TODO:
- notice there are still codes for ca inside phone.module, maybe implement hook for those.
p/s: someone ought to use coder to clean up the code/spaces/line ending, easier to generate patch as IDE has been configured according to Drupal coder when saving file.
#4
Hmm.. a bump, I think this should be checked in, it is troublesome to keep modifying just to add some local phone support.
#5
Reroll patch againsts HEAD.
Changes:
- Fully moved all country specific codes to their own inc file (incl. int)
- Added (where CC is the country code)
phone_CC_metadata
phone_CC_number_settings
phone_CC_number_validate
phone_CC_number_save
#6
Good work. I've built on that further more in ticket #613606: refactoring and simplification suggesition , adding sub-module auto-discovery, and reflection.
More work is needed with the architecture of this module. For example function phone_field_settings($op, $field) creates and handles help entries and special fields for certain country selections (currently int and ca). This functionality should be moved to the respective country sub-modules.
#7
All country specific codes have been moved into their own inc file in #5