HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

PDO prepared statement - binding variable number of values

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
numberstatementpreparedbindingvaluesvariablepdo

Problem

Please let me know if I have over/under explained my question :)

HTML table row - the numerals from each id attribute, in this example "408" and "409", are the database tables primary ID numbers, one database table row per HTML table cell.


    
    
    Delete


jQuery - I push these ids to an array and POST it to ajax.php

$('tbody').on('click','.deleteTableRow',function(){
var IDs = [];
$(this).closest('tr').find('td').each(function(){ IDs.push(this.id); });
var deleteTableRow = IDs;
$(this).parent().parent().remove();
    jQuery.ajax({
        type: "POST",
        url: "includes/ajax.php",
        data: {deleteTableRow: deleteTableRow},
        success: function (response) {            
            $('body').append(response);
        },
        error: function (response) {
            alert("Error getting php file");
        }
    });
});


PHP - ajax.php. This works wonderfully, but is there a better way to bind a variable number of values into a single prepared statement? Currently I have HTML tables containing 2 MySql rows or 3 MySql Rows, but this will need to handle around a maximum of 10 MySql rows

```
if(isset($_POST['deleteTableRow'])){

// Remove empty array value generated by the table cell containing my delete button
$deleteRowArray = array_filter($_POST['deleteTableRow']);

// Create an array of '?,' values - one for each id
foreach($deleteRowArray as $addBind) {
$array[] = '?,';
}

// Remove the last array values comma
end($array);
$array[key($array)] = '?';
reset($array);

// Create the finished query with as many binded values as needed
$sql = 'DELETE FROM dataformset WHERE DataFormSetId IN (' . implode($array) .') LIMIT 3';
$q = $db->prepare($sql);

$i = 1;
foreach($deleteRowArray as $deleteRow) {

// Remove the text "FieldId" and bind each ID into a single prepared statement
$deleteRow = preg_replace("/[^0-9,.]/", "", $deleteRow);

Solution

-
For this:

// Create an array of '?,' values - one for each id
foreach($deleteRowArray as $addBind) {
    $array[] = '?,';
}

// Remove the last array values comma
end($array);
$array[key($array)] = '?';
reset($array);

// Create the finished query with as many binded values as needed
$sql = 'DELETE FROM dataformset WHERE DataFormSetId IN (' . implode($array) .') LIMIT 3';


I guess the following is the same but much simpler:

$array = array_fill(0, sizeof($deleteRowArray), "?");
$sql = 'DELETE FROM dataformset WHERE DataFormSetId IN (' . implode(", ", $array) .') LIMIT 3';


You don't need to put the separator char into the array, implode can handle it and it doesn't put separator after the last element, so you don't have to remove it.

-

// Remove the text "FieldId" and bind each ID into a single prepared statement
$deleteRow = preg_replace("/[^0-9,.]/", "", $deleteRow);


I'd be a little bit more defensive here and remove only FieldId from the beginning of the string, not every non-allowed character. It's a sign of an error of the caller if you got a $deleteRow which doesn't start with FieldId. In that case you should indicate it somehow (throw an exception). There is no point to delete rows if the data is definitely invalid. (The Pragmatic Programmer: From Journeyman to Master by Andrew Hunt and David Thomas: Dead Programs Tell No Lies.)

Code Snippets

// Create an array of '?,' values - one for each id
foreach($deleteRowArray as $addBind) {
    $array[] = '?,';
}

// Remove the last array values comma
end($array);
$array[key($array)] = '?';
reset($array);

// Create the finished query with as many binded values as needed
$sql = 'DELETE FROM dataformset WHERE DataFormSetId IN (' . implode($array) .') LIMIT 3';
$array = array_fill(0, sizeof($deleteRowArray), "?");
$sql = 'DELETE FROM dataformset WHERE DataFormSetId IN (' . implode(", ", $array) .') LIMIT 3';
// Remove the text "FieldId" and bind each ID into a single prepared statement
$deleteRow = preg_replace("/[^0-9,.]/", "", $deleteRow);

Context

StackExchange Code Review Q#44930, answer score: 2

Revisions (0)

No revisions yet.