Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

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.

<tr>
    <td id="FieldId408"><!-- input element --></td>
    <td id="FieldId409"><!-- input element --></td>
    <td class="actionList"><a class="deleteTableRow">Delete</a></td>
</tr>

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);      
        $q->bindValue($i, $deleteRow, PDO::PARAM_STR);      
        ++$i;
    }   
$q->execute();
}
share|improve this question

1 Answer 1

up vote 2 down vote accepted
  1. 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.

  2. // 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.)

share|improve this answer
1  
Exactly what I was looking for! I don't think it could get much simpler than this. Cheers! I have used $deleteRow = str_replace('FieldId', '', $deleteRow); –  misterManSam Mar 21 at 5:28

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.