0

I am fairly new to prepared statements and am in the process of transitioning a project...

The last piece I have to transition is a piece where I have to update multiple rows/records.

This seems to work for me... However, I am curious and wondering about my technique and also sending back some sort of response (boolean or other) that everything was a success or failure. Thoughts? Comments? Suggestions?

function timeUpdate($uID, $galArr, $timeStamp) {
    global $mysqli; //my connection is set elsewhere (bad/good?)
    $q = "UPDATE someTable SET timeStamp = ? WHERE galleryID = ? AND uniID = ?";
    $stmt = $mysqli->prepare($q);
    $stmt->bind_param("iii", $timeStamp, $gID, $uID);

    foreach($galArr as $value) {
        $gID = $value[0];
        if(!$stmt->execute()) {
             throw new Exception($stmt->error, $stmt->errno);
        }
    }
    $stmt->close();
}

Thanks in advance. Any links, suggestions are appreciate.

Cool Shape
  • 309
  • 4
  • 18

1 Answers1

0
  1. It is always recommended to pass your connection as a function parameter instead of using global. You can research the reasoning for this if you like, or just trust my advice -- there will be a few StackOverflow pages dedicated to this matter.

  2. In the great majority of cases, a function should return something. In your case, I think it makes sense to return an array of success / failure outcomes; or if you have another specific need, you can return whatever you like.

  3. Your use of a prepared statement and bound parameters is wise/correct. I would just like to explain that it doesn't need $gID = $value[0]; if you simply write foreach(array_column($galArr) as $gID){. This declaration will be enough to feed the variable to execute(). That said, if you can prepare and pass the first column data from $galArr to your custom function, you will have a cleaner bit of code in your function... (foreach($gIDs as $gID){).

  4. Least important: I prefer not to declare single-use variables. In accord with this principle, I would not declare $q and opt to simply write the string into prepare(). (this is no big deal -- you can ignore this suggestion if you like)


Individualized queries for improved outcome tracking:

function timeUpdate($mysqli, $uID, $galArr, $timeStamp) {
    if(!$stmt=$mysqli->prepare('UPDATE someTable SET timeStamp = ? WHERE galleryID = ? AND uniID = ?')){
        return $result[]=['syntax'=>'fail','outcome'=>'Prepare Error: '.$mysqli->error];
    }elseif(!$stmt->bind_param("iii", $timeStamp, $gID, $uID)){
        $result[]=['syntax'=>'fail','outcome'=>'Bind Error: '.$stmt->error];
    }else{
        foreach(array_column($galArr,0) as $gID) {  // if these values are not unique, misinformation will be possible because I am using $gID as $result keys
            if(!$stmt->execute()) {
                $result[$gID]=['syntax'=>'fail','outcome'=>'Error: '.$stmt->error];
            }else{
                $result[$gID]=['syntax'=>'pass','outcome'=>'Affected Rows: '.$stmt->num_rows()];
            }
        }
    }
    $stmt->close();
    return $result;
}

Best practice would dictate that you make the fewest possible queries for a given task. Your task could be combined into a single UPDATE query using a WHERE / IN clause. Unfortuntely mysqli's prepared statements require a convoluted process when handling a variable number of placeholders. See this post for what that might look like. (PDO is much better equipped to handle variable data.) So basically, I'm saying there is a trade-off if you want to make a single call to the database -- you'll have to write a lot more (harder to read) code and you will have far less control of the the pass/fail message that you return.

mickmackusa
  • 43,625
  • 12
  • 83
  • 136