Valhalla Legends Forums Archive | Web Development | [PHP] Better way to do this PLEASE!

AuthorMessageTime
AndyOk, I'm trying to save a lot of settings. I've written a function called SaveSetting, which takes in an Account ID, a setting name, and the value. It returns 1 for saved, 2 for account does not exist, and a string containing an error message if MySQL errors. I want to save 11 settings, and check for errors every time. If any errors occur, I want it to stop saving and display the error message. If all the settings save, I want it to display a "Settings Saved" message.

This is the crappy way I've got so far:
Code:
     while ($result==1)
     {
      $result = SaveSetting($acct, 'email', $email);
      $result = SaveSetting($acct, 'aim', $aim);
      $result = SaveSetting($acct, 'msn', $msn);
      $result = SaveSetting($acct, 'icq', $icq);
      $result = SaveSetting($acct, 'yim', $yim);
      $result = SaveSetting($acct, 'bnet', $bnet);
      $result = SaveSetting($acct, 'loc', $loc);
      $result = SaveSetting($acct, 'descr', $descr);
      $result = SaveSetting($acct, 'birthdate', $bd);
      $result = SaveSetting($acct, 'gender', $gender);
      $result = SaveSetting($acct, 'signature', $sig);
      break;
     }
     if ($result==2)
     {
      $errID="username";
      include("forms/errors.php");
      $lnkID="home";
      include("forms/links.php");
     }
     else if($result==1)
     {
      $infID="profile";
      include("forms/infos.php");
      $lnkID="home";
      include("forms/links.php");
     }
     else
     {
      $errID="sql";
      $err=$result;
      include("forms/errors.php");
      $lnkID="home";
      include("forms/links.php");
     }

This can't be the right way to do this. Can someone please help me out?
April 16, 2007, 01:04 AM
ErsanYour SaveSetting function sucks, something like this will do what you want:
Code:
function SaveSettings($settings, $acct) {
// Do whatever you do to make sure the "acct" is valid here...
foreach($settings as $key => $value) {
mysql_query("UPDATE settings SET $key = $value WHERE account = $acct") or die("Error saving setting '$key'");
}
print("Settings saved.");
}

calling:
Code:
$settings['email'] = "emailaddr@whatever.com";
$settings['lamer'] = "yes";
SaveSettings($settings, $acct);

But honestly you should be trying to save them all in the same query, and account exists should definitely be done seperately...  Otherwise you're dishing out unnecessary overhead.  I'm also guessing on your database structure but that's what a sane person would use...
April 16, 2007, 04:01 AM
AndyI guess a dynamically created query might be a better idea.

My SaveSetting function is:
Code:
function SaveSetting($id,$setting,$value)
 {
  $query="SELECT * FROM Accounts WHERE id = '$id'";
  $ids=mysql_query($query);
  $idnum=mysql_numrows($ids);
  if ($idnum != 1)
  {
   return 2;
  }
  else
  {
   $query="UPDATE Accounts SET $setting = '$value' WHERE id = '$id'";
   $result=mysql_query($query);
   if ($result==1)
   {
    return 1;
   }
   else
   {
    return mysql_error();
   }
  }
 }
April 16, 2007, 04:40 AM
ErsanDude you really should learn how mysql works, use the function I posted instead of yours, and if you ever want to get the number of rows w/o doing anything else with that data use SELECT COUNT(*) FROM Accounts WHERE id = '$id' (but again you shouldn't be doing that).April 16, 2007, 04:52 PM
AndyOk... Is there any special way to pass an array to a function?April 16, 2007, 05:04 PM
ErsanNope, php is typelessApril 16, 2007, 05:06 PM
AndyLet's say I do a GetSettings function like this:
Code:
function GetSettings($id, $settings)
 {
  $query = "SELECT * FROM Accounts WHERE id = '$id'";
  $ids = mysql_query($query);
  $idnum = mysql_numrows($ids);
  if ($idnum != 1)
  {
   return "noacc";
  }
  else
  {
   for($i = 0; $i < count($settings); $i++)
   {
    $setting = strtolower($settings[$i]);
    $result = mysql_result($ids, 0, "$setting");
    $results[] = $result;
   }
   return $results;
  }
 }
And then call it to get settings like this:
Code:
$settings = array('username', 'posts', 'regdate', 'email', 'aim', 'msn', 'icq', 'yim', 'bnet', 'loc', 'descr', 'birthdate', 'gender', 'signature', 'flags');
 List($username, $posts, $registered, $email, $aim, $msn, $icq, $yim, $bnet, $loc, $descr, $bday, $gender, $sig, $flags) = GetSettings($acct, $settings);

Anything I'm doing inefficiently here?

Edit: I suppose I could combine     $result = mysql_result($ids, 0, "$setting");    $results[] = $result;
April 16, 2007, 05:33 PM
Ersan
Code:
function GetSettings($id, $settings)
 {
  $ids = mysql_query("SELECT * FROM Accounts WHERE id = '$id'");
  if (mysql_num_rows($ids) != 1)
  {
   return "noacc";
  }
  else
  {
   foreach($settings as $setting)
   {
    $results[] = mysql_result($ids, 0, strtolower($setting));
   }
   return $results;
  }
 }
April 16, 2007, 06:02 PM