We should use drupal_static() in _country_get_predefined_list() too.
I've right now a case where I need to fetch the country list in different languages in the same request.
And I think the easiest - if even not the fastest way - is to temporarily change the $language global, reset the static cache and fetch the list again.
If there's a smarter way to deal with that - suggestions are welcome :)
Nevertheless we should change the static handling here - if nothing essential speaks against it.
| Comment | File | Size | Author |
|---|---|---|---|
| #1 | drupal-iso-country-list-use-drupal_static-and-accept-langcode-param-1069320-1.patch | 20.51 KB | das-peter |
| drupal-iso-country-list-use-drupal_static.patch | 423 bytes | das-peter |
Comments
Comment #1
das-peter commentedI refactored this again after thinking more about it.
My conclusion was that a function that returns translated strings should accept a language parameter.
Is there no coding standard / guideline rule for cases like this?
Attached patch contains now the usage of
drupal_static()and the handling for the$langcodeparameter.Comment #2
moshe weitzman commentedIf you want to add caching, we need justification via benchmarks.
Comment #3
das-peter commentedThanks for your feedback Moshe.
This is not really about caching - it's more about localization / translation.
If the
$langcodeparameter is added,drupal_static()probably can be removed.But since I thought the usage of
drupal_static()is encouraged, I kept it.My intention was to be able to get the country names in a specific language - and I think every function that returns translated stuff should provide a way to define the needed language and not only rely on the
$languageglobal.Comment #4
gábor hojtsyNew features are added to Drupal 8.
Comment #5
gábor hojtsyAlso, the function seems to be a private one (it starts with an underscore) and therefore probably not intended to be used from elsewhere. I think you can do a wrapper in D7 contrib by setting the language temporarily to something else, and you can also cache results thee. For Drupal 8, if we are going to expose this to the world, we should rename the function IMHO to reflect that decision.
Comment #6
sunThis appears to be obsolete in D8 — although I could be mistaken. Please re-open if I'm wrong.