0

i have created a query class to handle all my basic sql functions, inside the class i have a basic function that inserts data using prepared statements that is the 'Insert' function, am not quite sure why but i keep getting the same error(listed above) every time i call the function


class Query
{

    private $conn;
    private $table;

    public function __construct($conn, $table)
    {
        $this->conn = $conn;
        $this->table = $table;
    }

     public function Insert($fields, $placeholders, $binders, $values)
     {
        $field_val= implode(', ', $fields);
        $ph=implode(', ', $placeholders);

        array_walk($values, function(&$x) {$x = "'$x'";});

        $val = implode(',  ', $values);

        $query = 'INSERT INTO '.$this->table.' ('.$field_val.') VALUES('.$ph.')';

        $stmt = $this->conn->prepare($query);

        $stmt->bind_param(''.$binders.'', $val);

        $stmt->execute(); 

     }
}

this is an example of the function in use to insert data into a comments table, the connection comes form a separate database file included in the module

    $database = new Database();
    $conn = $database->connect();

    //values
    $comment_date = mysqli_real_escape_string($conn, htmlspecialchars($_POST['date-comment']));
    $commenter = mysqli_real_escape_string($conn, htmlspecialchars($_POST['comment_name_of']));
    $comment = mysqli_real_escape_string($conn, htmlspecialchars($_POST['comment']));

    $comments_save = new Query($conn, 'nw_comments');
    $fields = array('commenter_name', 'comment_value', 'date_commented');
    $placeholders = array('?', '?', '?');
    $binders = "sss";
    $values = array($commenter, $comment, $comment_date);
    $comments_save->Insert($fields,$placeholders,$binders,$values);
Dharman
  • 30,962
  • 25
  • 85
  • 135
Fifth Horseman
  • 128
  • 12
  • Don't use both real_escape_string and prepare. Drop the real_escape_string all together as prepare is more secure. – aynber Mar 03 '20 at 18:18
  • Echo/`var_dump($query)`, and you should be able to see exactly what is wrong. `array_walk($values, function(&$x) {$x = "'$x'";});` is completely unnecessary and probably screwing things up. – aynber Mar 03 '20 at 18:19
  • i did that but the error is still persistent – Fifth Horseman Mar 03 '20 at 18:20
  • `$stmt->bind_param(''.$binders.'', $val);` $val needs to be separate variables, not a single variable or string. – aynber Mar 03 '20 at 18:22
  • i use array_walk($values, function(&$x) {$x = "'$x'";}); to add quotes to the values since implode supposedly failed to do it – Fifth Horseman Mar 03 '20 at 18:23
  • 2
    Try `$stmt->bind_param(''.$binders.'', ...$values);` – Nigel Ren Mar 03 '20 at 18:30
  • i removed the array_walk function and var_dumped $val containing sample input values from a form and this is how the values look like string(49) "jonte, kal sanders, 2020-03-03 09:19:54PM +0300", i think the problem is the quotes on individual values – Fifth Horseman Mar 03 '20 at 18:32
  • Nigel, your answer is incredible, it worked, can you explain how? – Fifth Horseman Mar 03 '20 at 18:34
  • https://stackoverflow.com/a/36071202/1213708 may explain it. – Nigel Ren Mar 03 '20 at 18:36

1 Answers1

0

Looks like the code is passing a string scalar to bind_param. I think we want to pass the elements of the array.

We can do something like this:

$stmt->bind_param(''.$binders.'', $values);

As Bill Karwin correctly points out, passing the entire array (as in the line above) won't work, it has the same problem as the original passing a single reference. The ... syntax should cause that array to be unpacked, each individual element passed by reference, a variable number of elements.)

Something like this:

$stmt->bind_param(''.$binders.'', ...$values);

and for passing values through bind parameters, we don't need to enclose the values in single quotes. These lines are not needed: 

 array_walk($values, function(&$x) {$x = "'$x'";});
 $val = implode(',  ', $values);
spencer7593
  • 106,611
  • 15
  • 112
  • 140
  • changing to $stmt->bind_param(''.$binders.'', $values); still brings the error and the table is still empty – Fifth Horseman Mar 03 '20 at 18:27
  • 1
    `bind_param()` takes a variable number of arguments, and the arguments must be passed by reference. I don't think you can just pass an array and have it work the same. This is one example where PDO is much easier, because you can pass an array to `PDOStatement::execute()`. – Bill Karwin Mar 03 '20 at 18:53
  • 1
    You can make the array work by using the `...` syntax to turn an array into variable-arguments. See documentation here: https://www.php.net/manual/en/functions.arguments.php#functions.variable-arg-list – Bill Karwin Mar 03 '20 at 19:04
  • @BillKarwin: excellent points. Please do feel free to make corrections and improvements to any of my answers. (Passing the whole array doesn't work, because that is passing just a single reference (*DOH!*). My answer edited to implement the suggested `...` syntax to convert the array into a variable number of arguments. – spencer7593 Mar 03 '20 at 20:04
  • 2
    I would just use PDO. Life is too short to waste my time on `mysqli_stmt_bind_param()`! :-) – Bill Karwin Mar 03 '20 at 21:44