Performing redirects based on taxonomy terms associated with nodes

This code performs 301 redirects based on the taxonomy terms associated with the node, as 301 redirects can adversely effect page load time. How could this be best optimized?

```
taxonomy);//nstores number of key/value pairs in array, if two terms are selected with the node/post then count == 2

$terms = array();//initialize terms to array
foreach($node->taxonomy as $term) {
$terms[$term->name] = $term->name; //prints array of individual terms

}

/case 1 new england/

//if there are less than 9 terms and more than 2 and new england is in the array redirect to new england
if( $count 2 && in_array('newengland', $terms) && $region != 'newengland')
{
$link = 'http://newengland.thedelimagazine.com/' . drupal_get_path_alias("node/$node->nid", $path_language = '');
drupal_goto($link, $query = NULL, $fragment = NULL, $http_response_code = 301);
}
/END case 1 new england/

/case 2/
if($count == 2 && in_array('national', $terms) && is_numeric(arg(1))) { //if there are 2 terms in taxonomy && national is one of them & the first argument in url is a number such as thedelimagazine.com/12345 (before alias redirect)
unset($terms['national']); //remove national from array
$value = array_shift($terms) ;//set value to the other term left. in array
if($value == 'los angeles') {//fix for los angelas, because in deli module taxonomy term and subdomain are different only for los angeles
$value = 'la';
}

if($region != $value) { //if region does not already = term (would cause redirect loop)

$link = 'http://' . $value . '.thedelimagazine.com/' . drupal_get_path_alias("node/$node->nid", $path_language = '');//go to term left in $value

drupal_goto($link, $query = NULL, $fragment = NULL, $http_respo

function region_filter_nodeapi(&$node, $op, $a3 = NULL, $a4 = NULL) {

      switch ($op) {

        case 'view':

            if( (arg(1) == 0)){ } //do not execute on homepage. if there are no aruments after the base url in arument one is thedelimagazine.com/argument1, if this is 0 then do nothing.

You get an extra pair of parens, and it would be easier to to check for arg(1) ~= 0 rathern the have an empty if block.

else {

            $region = getRegion(); //get current region from deli.mmodule

            $national = 'national'; //store string 'national' in variable

Don't write dumb comments. Assume the reader understands PHP. Your comment should explain why you want to do that, not reiterate the code.

$count = count($node->taxonomy);//nstores number of key/value pairs in array,  if two terms are selected with the node/post then count == 2

            $terms = array();//initialize terms to array
            foreach($node->taxonomy as $term) {    
              $terms[$term->name] = $term->name; //prints array of individual terms

            }

That was a lot of pointless empty lines.

/*case 1 new england*/

        //if there are less than 9 terms  and more than 2 and new england is in the array redirect to new england
        if( $count  2 && in_array('newengland', $terms) && $region != 'newengland')

Again, your comment simply tells me what I could have already gotton from the code. But I'm left thinking: WHAT? What's the deal with the number of terms? What am I checking in the array? What has $region got to do with it?

{
          $link = 'http://newengland.thedelimagazine.com/' . drupal_get_path_alias("node/$node->nid", $path_language = '');
              drupal_goto($link, $query = NULL, $fragment = NULL, $http_response_code = 301);

Keep your indentation consistent.
}
/END case 1 new england/

Don't put end comment like that. I can already see the case is ending thanks to the } and your indentation.

/*case 2*/
              if($count == 2 && in_array('national', $terms) && is_numeric(arg(1))) { //if there are 2 terms in taxonomy && national is one of them & the first argument in url is a number such as thedelimagazine.com/12345 (before alias redirect)

Again, don't repeat your code in the comments. Explain why your code is doing that.

unset($terms['national']);        //remove national from array
              $value = array_shift($terms) ;//set value to the other term left. in array 
              if($value == 'los angeles') {//fix for los angelas, because in deli module taxonomy term and subdomain are different only for los angeles

See those are the comments that are helpful, because it explains what so special about los agenles.

$value = 'la';
              }      

              if($region != $value) { //if region does not already = term (would cause redirect loop)

Here I'd just say // avoid redirect loop

$link = 'http://' . $value . '.thedelimagazine.com/' . drupal_get_path_alias("node/$node->nid", $path_language = '');//go to term left in $value

                  drupal_goto($link, $query = NULL, $fragment = NULL, $http_response_code = 301);//drupal redirect function

This looks pretty much the same as the last case. You should refactor the code.
}

}

/* ++ END case 2++*/

            /* ++case 3 national+++*/ 
            else {

        if($count > 10  && in_array('national', $terms)  && $region != 'national' ) {    //if there are moer than 10 terms and national is in the array $terms, and the current region does not already =national           
              $link = 'http://national.thedelimagazine.com/' . drupal_get_path_alias("node/$node->nid", $path_language = '');//then set $link to national.thedelimagazine.com.  drupl get path alias gets the path to the node and node id and converts it to clean readable urls that i implemented last year.
              drupal_goto($link, $query = NULL, $fragment = NULL, $http_response_code = 301);     //go to the $link
        }
             /* ++ END case 3 national++*/

             }
         }
          break;
      }
    }

It's been so long so I read the matching pieces of code that I have no idea what these pieces much up to. That a sign that your code is overly complicated.

Here is a quick reworking of the code:

```
taxonomy);

// I doubt this is neccesary, I don't know what
// $node->taxonomy is, but perhaps you can use that rather
// then copying the data into an array.
$terms = array();
foreach($node->taxonomy as $term) {
$terms[$term->name] = $term->name;
}

// I really have no idea what the logic behind the region is
if($taxonomy_count == 2 && in_array('national', $terms) && is_numeric(arg(1)))
{
$region = array_shift($terms);
// los ageneles needs special case

StackExchange Code Review Q#7345, answer score: 2