Overridable Theming Functions

In the code, I have noticed a somewhat problematic use of theming functions. Although it is not the only place where this occurs, here's the instance which caught my eye.

The file sopac.module contains the theme_sopac_search_handler function. The comment for this function indicates, "It very well may be that this function needs to be overridden due to themeing considerations, so it takes advantage of Drupal's hook_theme and may be defined in template.php."

However, the theme_sopac_search_handler function is invoked in a way which does not work with overriding. In the sopac_menu function, the search_handler item includes the following:

'page callback' => 'theme_sopac_search_handler,

This call will always call the theme_sopac_search_handler function, bypassing any attempts to override it. To enable overriding, the menu declaration should instead include this:

'page callback' => 'theme',
'page arguments' => array('sopac_search_handler'),

This will result in a call to theme('sopac_search_handler'), and that will call either theme_sopac_search_handler or the overriding function with the highest priority.

As a final note, I would suggest that it is preferable to avoid naming a function with the theme prefix unless one is going to use the theme function to invoke it. I suggest this because using the theme prefix implies that the function can be overridden, but invoking it by means other than the theme function prevents overriding from working.