Though a call to module_exists() is not very expensive, in a tight loop like this, it might be an idea to store the result of that call into a local variable that gets initialized outside the loop:

function _hs_taxonomy_hierarchical_select_terms_to_options($terms) {
  $options = array();
  $use_i18n = module_exists('i18n_taxonomy');
  foreach ($terms as $key => $term) {
    // Use the translated term when available!
    $options[$term->tid] = $use_i18n ? i18n_taxonomy_term_name($term) : t($term->name);
  }
  return $options;
}

Even better would be to get all translations at once, but there does not yet seem to be a function in i18n that does so. there are some _multiple_ functions over there, but none that only executes one query.

CommentFileSizeAuthor
#1 1220830-1.patch631 byteswim leers

Comments

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: Easy way to Increase performance of _hs_taxonomy_hierarchical_select_terms_to_options » Minor performance improvement for _hs_taxonomy_hierarchical_select_terms_to_options()
Category: feature » task
Status: Active » Needs work
StatusFileSize
new 1220830-1.patch631 bytes

module_exists() uses module_list(), which has a cached list. Nevertheless, this is indeed a minor performance improvement.

However, you're using i18n_taxonomy_term_name() instead of the current code's tt(). Which is it? Partial patch attached.

fietserwin’s picture

fietserwin
Location Le Mont-Dore
commented

I don't know tt, I guess that one is from i18n_string or D6? i18n_taxonomy offers a more specialized function name: i18n_taxonomy_term_name(). So that looks like the one to use, especially as tt seems to require knowledge about how i18n_taxnomy formats the context field in the locale tables.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

So you're certain that this is safe to commit? Because it requires too much additional work to set up a i18n testing site — I rely on reports only to make sure this works.

fietserwin’s picture

fietserwin
Location Le Mont-Dore
commented

I see, but how can someone from Belgium not have multilingual installs? :)

Anyway, I did some research into the tt() function and found this in a D6 install:

file: i18nstrings..module:

/**
 * Translate or update user defined string.
 *
 * DEPRECATED, just kept for backwards compatibility.
 *
 * @todo Remove tt() for Drupal 7.
 * @see i18nstrings()
 */
function tt($name, $string, $langcode = NULL) {
  return i18nstrings($name, $string, $langcode);
}

I couldn't find any reference to it in D7 i18n. So yes, I'm sure you should use the i18n_taonomy_term_name:
file: i18n_taxonomy.module

/**
 * Get localized term name
 */
function i18n_taxonomy_term_name($term, $langcode = NULL) {
  return i18n_taxonomy_vocabulary_mode($term->vid, I18N_MODE_LOCALIZE) ? i18n_string(array('taxonomy', 'term', $term->tid, 'name'), $term->name, array('langcode' => $langcode)) : $term->name;
}

So this function even does more than just looking up. It looks like your D6 code contains an error here: trying to translate terms from not localized vocabularies? (or probably not an error but at least a performance hit, on the other hand: perhaps some remains are still there in the locale tables giving false results)

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs work » Closed (duplicate)

Because I avoid both Dutch & French and prefer English? :) It's a valid question though :)

This has now already been committed through #1216214: D7 i18n Taxonomy compatibility :)