By patricio.keilty on

The following code snippet is from ecommerce/product/product.module:


function product_menu($may_cache) {
  ...
  if ($may_cache) {
    ...
  }
  else {
    foreach(product_get_ptypes() as $type => $description) {
      $items[] = array(
        'path' => 'admin/settings/content-types/product/' . $type,
        'title' => $description,
        'callback' => 'product_types_configure',
        'access' => user_access('administer products'),
        'type' => MENU_CALLBACK
      );
    }
  }
  ...

Could someone please explain me the rationale behind the code in the else block?, seems to be building the special menu items for each product type....
This code is non-cached and is called on *every request* to drupal site, which seems not necessary to me, could we gain in performance if moved to the cached portion? What are the dis/advantages in this case?

Thanks in advance,
--p

⋅ Categories: Drupal 4.7.x

Comments

rszrama’s picture

rszrama commented

You don't want menu items cached that may change. For this case, a store owner may add or remove product types, but Drupal wouldn't pick up on the changes. That's why things like node links, edit pages, user pages, etc. are not cached.

This loop seems trivial to me... unless you have thousands of product types I doubt it's slowing you down much. All this is doing is defining variables, and that's done blazingly fast in the grand scheme of things.

----------------------
Current Drupal project: http://www.ubercart.org

patricio.keilty’s picture

patricio.keilty commented

Good point Ryan.... anyway seems other modules are using some conditionals to optimize a little bit, e.g. node.module does some checks on args before hitting the db

  ...
  if ($may_cache) {
  ...
  }
  else {
    if (arg(0) == 'node' && is_numeric(arg(1))) {
      $node = node_load(arg(1));
      if ($node->nid) {
   ...

Can you recommend any approach to profile/measure drupal code, e.g. PHP/MySQL profiler? drupal module? custom code? whatever works for you...

Thanks in advance,
--p