[PHP] Better way to do this PLEASE!

[Barabajagal]

Ok, 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 Select Expand


     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?

[Ersan]

Your SaveSetting function sucks, something like this will do what you want:

Code Select Expand


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 Select Expand


$settings['email'] = "[email protected]";
$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...

[Barabajagal]

I guess a dynamically created query might be a better idea.

My SaveSetting function is:

Code Select Expand

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();
   }
  }
}

[Ersan]

Dude 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).

[Barabajagal]

Ok... Is there any special way to pass an array to a function?

[Ersan]

Nope, php is typeless

[Barabajagal]

Let's say I do a GetSettings function like this:

Code Select Expand

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 Select Expand

$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;

[Ersan]

Code Select Expand

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;
  }
}