incorrect use of drupal_alter

The purpose of drupal_alter is to pass a variable (array or object) that can be manipulated by other modules. Also with D7 there is a general rule of removing $op function variables in functions in favour of separate functions for each $op.

see http://api.drupal.org/api/function/drupal_alter/7

The drupal_alter function in question is:

function hook_tapir_table_header_alter(&$header, $table_id)

// uc_product.module on line 1287
drupal_alter('tapir_table_header', $columns, 'uc_product_table'); 

Where the 2nd argument is being used as a $op variable to select a specific table.

There is also another alter function defined in the api that implements drupal_alter incorrectly:

function hook_tapir_table_alter(&$table, $table_id)

The solution here should be to change the alter function to something like:

function hook_tapir_uc_product_table_header_alter(&$header) 

Something like this?

hook_tapir_table_alter() was based off of hook_form_alter(), and I don't think the $form_id parameter is ever going to go away. It's interesting, though, that this means that $form_id is passed by reference to hook_form_alter(). Don't anyone take advantage of that, though.

If you want table-specific table alter hooks, it's easy to add them in. However, the proper solution for this issue is to just put 'uc_product_table' into a variable, and pass that to drupal_alter() instead. That means far fewer changes across the board, and we're still doing things the Drupal way.

Don't anyone take advantage of that, though.

If you just create a variable for the purpose of passing this information it wont make a different what anyone does with it.

Rereading the docs: http://api.drupal.org/api/function/drupal_alter/7 it does talk about passing variables as a $context, so I guess your solution of making it a variable is fine.

Made this change as I was testing the port. Thanks for the early warning.