is this a smart way to write this function?

[drewbie]

i added a switch stament to my function so i can call the function from 2 different pages and grab info from the database from either the current comments or history comments. is this a good way to do this so i dont have to write multiple functions that do basically the same thing? any tips/suggestions are appreciated. thanks!

function add_remark($call_id, $remark, $importance = "0")
{
   global $link_id, $login;

   switch($_SERVER['PHP_SELF']){
      case '/abm_test/cur_all.php':
         $field_name = 'CID';
         $table_name = 'current_calls';
         $comments_table_name = 'comments_calls';
         break;
      case '/abm_test/his_all.php':
         $field_name = 'CID';
         $table_name = 'call_his';
         $comments_table_name = 'comments_calls2';
         break;
   }
   $check_query = mysql_query("SELECT $field_name FROM $table_name WHERE $field_name='$call_id'", $link_id) or die(mysql_error());
   $check_num_rows = mysql_num_rows($check_query);
   if ($check_num_rows > 0)
   {
      $unique_id = $_SESSION['comment_key'];
      $unique_id = substr($unique_id, 0, 30);
      $SESSION_ID = session_id();
      $SESSION_ID = substr($SESSION_ID, 0, 30);
      $query = mysql_query("SELECT CCID FROM $comments_table_name WHERE sess_id='$SESSION_ID' AND unique_id='$unique_id'", $link_id) or die(mysql_error());
      $total = mysql_num_rows($query);
      if ($total == 0)
      {
         do
         {
            $ccid = md5 (uniqid (rand()));
            $results = mysql_query("SELECT CCID FROM comments_calls WHERE CCID='$ccid'", $link_id) or die(mysql_error());
            $results2 = mysql_query("SELECT CCID FROM comments_calls2 WHERE CCID='$ccid'", $link_id) or die(mysql_error());
            if(!$results)
            {
               echo "Query $query failed.";
               echo mysql_error();
            }
            if(!$results2)
            {
               echo "Query $query failed.";
               echo mysql_error();
            }
            $num_rows = mysql_num_rows($results);
            $num_rows2 = mysql_num_rows($results2);
            $num_rows = $num_rows + $num_rows2;
         } while ($num_rows == 1);
         $remark = htmlspecialchars($remark);
         $user_id = $login->userinfo['id'];
         $comment_query = mysql_query("INSERT INTO $comments_table_name VALUES('$ccid', '$call_id', '$user_id', '$remark', '0', '$SESSION_ID', '$unique_id', NOW() + 0, '$importance')", $link_id) or die(mysql_error());
         unset($_SESSION['comment_key']);
         return TRUE;
      }
      else
      {
         return FALSE;
      }
   }
   else
   {
      return FALSE;
   }
}

[tbach2]

do this so i dont have to write multiple functions

Sounds like you're developing the prized virtue of laziness. That's good! Don't let it stop there. IMHO you need to apply more laziness to the function. I think if you cultivate enough laziness, you probably will not need the switch statement, though.

For example: you have a very creative way to ensure that ccid is unique in the database; but there is a much lazier way to do it: autoincrement .

Also,

$results = mysql_query("SELECT CCID FROM comments_calls WHERE CCID='$ccid'", $link_id) or die(mysql_error());
$results2 = mysql_query("SELECT CCID FROM comments_calls2 WHERE CCID='$ccid'", $link_id) or die(mysql_error());
 

raises a red flag: what is the difference between them? current or history? wouldn't that be better served by one database with a timestamp field to determine what is current or history?

anyway, that's my $.02 worth...

HTH