insert to mysql using variable variables

cretaceous

I've never really understood if it's ok to do mysql inserts / updates by chunking through an array and using variable variables - I seem to remember Laserlight advised against it but I cant find the post

so here's my code - please comment!

// build array that I'll use for other things so ignore explicit titles
$variables = array(
'adm_fname' => 'First name',
'adm_lname' => 'Last name',
'adm_uname' => 'Username',

'adm_co' => 'Company',

'adm_priv' => 'Access level',
'adm_tel' => 'Telephone',
'adm_mobile' => 'Mobile',
'adm_email' => 'Email'
);

// now build sql strings

if ($adm_id) {
    $sql = "UPDATE admin_usr SET ";
    $v=0;
    foreach ($variables as $variable => $title) {
    if($v) $sql.= ', ';
     $sql.= "$variable = '".mysql_escape_string(trim($_REQUEST[$variable]))."' ";
    $v++;
    }
    $sql.=" WHERE adm_id=$adm_id";


  } else {

$sql = "INSERT INTO admin_usr ( ";

$v=0;
foreach ($variables as $variable => $title) {
    if($v) $sql.= ', ';
     $sql.= $variable;
    $v++;
  }
  $sql.= " ) VALUES ( ";

  $v=0;
  foreach ($variables as $variable => $title) {
    if($v) $sql.= ', ';
     $sql.= "'".mysql_escape_string(trim($_REQUEST[$variable]))."'";
    $v++;
  }
$sql.= " )";
          }

// then do the mysql query..

djjjozsi

its like working, but not safe.

and you get that $adm_id value ?

lets test if the user fill all the forms.

if it is an update form, there might have fields that are unique.

Weedpacket

I don't see any variable variables.

Incidentally, since you only ever use the array keys when you're looping

foreach(array_keys($variables) as $variable)

would save the redundant $title variable from floating around. And since you're using an array, there might be some array functions that would simplify what you're doing. E.g.,

$v=0;
foreach ($variables as $variable => $title) {
    if($v) $sql.= ', ';
     $sql.= $variable;
    $v++;
  }

could be replaced by

$sql .= join(", ", array_keys($variables));

and

$v=0;
foreach ($variables as $variable => $title) {
    if($v) $sql.= ', ';
     $sql.= "'".mysql_escape_string(trim($_REQUEST[$variable]))."'";
    $v++;
  }

$request_values = array_intersect_key($_REQUEST, $variables));
$request_values = array_map('mysql_escape_string',(array_map('trim', $request_values));
$request_values = array_map(create_function('$a','return "\'$a\'";'), $request_values);
$sql .= join(", ", $request_values);

although it would be tidier to have a distinct function to do that trimming, escaping, and quoting, and write

$sql .= join(', ', array_map('prep_variable', array_intersect_key($_REQUEST, $variables)));

And then of course there's PDO: then the escaping and quoting steps wouldn't be needed.

cretaceous

many thanks for replies

sorry djjjozi I was only doing a kind of pseudocode to show the structure - I fully intend to do user entry checking and in-depth validation with another function

Weed (if I may call you that ; ) ) - firstly I stupidly managed to edit out the bit with the variable variable

foreach ($variables as $variable => $title) {
$$curvar=  mysql_escape_string(trim($_REQUEST[$variable]));
}

that was to set each value explicitly but maybe I dont need that..

your other comments are worth their weight in gold - really appreciate it

I'm sick of typing lengthy strings of variables numerous times for updates/inserts and these are the steps I want to go through to make it into a function (and even work towards making it into a class - if I can just get my brain around writing those)