views:

122

answers:

3

Hello, I'm doing my first steps with mysql and php, so I have doubts on foundamental rules for a right code optimization.

I have a case where my UPDATE statement need to be executed on a certain number of rows, because it should be executed on a relational table, so is a for cicle correct?

<?
// connection already created
$data[] = array ("id" => 54, "enabled" => 1);
$data[] = array ("id" => 33, "enabled" => 0);
$data[] = array ("id" => 12, "enabled" => 0);
$data[] = array ("id" => 58, "enabled" => 0);
$data[] = array ("id" => 21, "enabled" => 1);
$data[] = array ("id" => 10, "enabled" => 1);
$data[] = array ("id" => 18, "enabled" => 0);
$data[] = array ("id" => 32, "enabled" => 1);
$data[] = array ("id" => 84, "enabled" => 0);
$data[] = array ("id" => 80, "enabled" => 1);

for (var $i = 0; $i < count ($data); $i ++) {

    $id = $data[$i]["id"];
    $enabled = $data[$i]["enabled"];

    $sql = "UPDATE users SET user_enabled = '$enabled' WHERE user_id = '$id' LIMIT 1;";
    $res = mysql_query ($sql);
    $num = mysql_num_rows ($res);

}


?>

Should I use a while or a for loop? Is this code valid for multiple UPDATEs or does exist something better like specific queries for this kind of action?

+3  A: 

With a sequence of finite length you should use a foreach loop.

Ignacio Vazquez-Abrams
A: 

you could use a RecursiveArrayIterator

<?
// connection already created
$data[] = array ("id" => 54, "enabled" => 1);
$data[] = array ("id" => 33, "enabled" => 0);
$data[] = array ("id" => 12, "enabled" => 0);
$data[] = array ("id" => 58, "enabled" => 0);
$data[] = array ("id" => 21, "enabled" => 1);
$data[] = array ("id" => 10, "enabled" => 1);
$data[] = array ("id" => 18, "enabled" => 0);
$data[] = array ("id" => 32, "enabled" => 1);
$data[] = array ("id" => 84, "enabled" => 0);
$data[] = array ("id" => 80, "enabled" => 1);

$data = new RecursiveArrayIterator($data);

for (var $i = 0; $i < count ($data); $i ++) {

    $id = $data[$i]["id"];
    $enabled = $data[$i]["enabled"];

    $sql = "UPDATE users SET user_enabled = '$enabled' WHERE user_id = '$id' LIMIT 1;";
    $res = mysql_query ($sql);
    $num = mysql_num_rows ($res);

}


?>
streetparade
+1  A: 

Several comments:

  • Calculating the count($data) on every iteration of the loop is a needless cost. Either start the loop at count($data) and count down, or else as @Ignacio suggests, use foreach.

  • This is a great example where it would be a benefit to use a prepared query with parameters. But this feature is not supported in PHP's plain mysql extension. You would have to use mysqli or preferably PDO.

  • You don't have to put quotes around integer values in an SQL expression.

  • mysql_num_rows() is meaningless for an UPDATE query. You probably meant to use mysql_affected_rows().

  • I don't know why you use LIMIT in this query, since I would guess that user_id is the primary key of the users table, and therefore you would never update more than one row anyway.

Here is how I would write that code:

<?php
// PDO connection already created
$data[] = array ("id" => 54, "enabled" => 1);
...etc...

$sql = "UPDATE users SET user_enabled = :enabled WHERE user_id = :id";
$stmt = $pdo->prepare($sql);

foreach ($data as $parameters) {

    $success = $stmt->execute($parameters);
    $num = $stmt->rowCount();

}


?>
Bill Karwin
I love this site simply for one thing, you learn
Vittorio Vittori