PDO Insert function using PHP

Question

This code inserts data into the database successfully. I was concerned about whether it is a considered good practise to go about inserting data into the database in this way. Is there something I am not considering?

function insertTest(){

    global $db;

    $field_names    = array("id", "first_name", "last_name", "email");
    $alias          = array(":id", ":fn", ":ln", ":em");
    $assignment     = array(":id" => "2k39dk38dk2k39dk38dk2k39dk38dk2k39dk38dk",
                            ":fn" => "Chris", 
                            ":ln" => "Moore", 
                            ":em" => "test@gmail.com");

    $array = array();
    $array['table'] = 'users';
    $array['field_names'] = $field_names;
    $array['alias'] = $alias;
    $array['assignment'] = $assignment;

    $q = "INSERT INTO `%s` (%s) VALUES (%s)";
    $q = sprintf($q, $array['table'], implode(", ",$array['field_names']), implode(", ",$array['alias']));
    $q = $db->prepare($q);
    $r = $q->execute($assignment);

    print_r($r);

    return $r;

}

Answer 1

Is it good practice to insert data into a table this way?

If by that you mean: is it considered good practice to use prepared statements, and a bind array, then yes. It is. If you mean: "can I consider my code to be good practice", then the answer is no. It's not. Here's why:

function insertTest()
{//needs to go on the next line, PSR standards
    global $db;

The use of functions + global keyword is bad practice. For starters: you have no real way of ensuring that $db will be an instance of PDO. And seeing as you are using a function, why not require the caller to pass a valid DB connection to your function:

function insertTest(PDO $db)
{
}

Now you know what $db will be, with the added benefit of the type-hint, any decent IDE will know, too. So your code is self-explanatory.

Next we have this:

$field_names    = array("id", "first_name", "last_name", "email");
$alias          = array(":id", ":fn", ":ln", ":em");
$assignment     = array(":id" => "2k39dk38dk2k39dk38dk2k39dk38dk2k39dk38dk",
                        ":fn" => "Chris", 
                        ":ln" => "Moore", 
                        ":em" => "test@gmail.com");

That looks a tad messy, doesn't it. For starters, it's pretty clear to me that $field_names and $alias belong together: for each field in your INSERT, there needs to be a placeholder. Having 2 separate arrays means that someone can add a value to the first, and simply forget about adding a placeholder in the other array. That would break your code. Just write:

$map = [
    'id'         => ':id',
    'first_name' => ':fn',
    'last_name'  => ':ln',
    'email'      => ':email',
];

That addresses that issue. Now you can easily extract the fields you're using (array_keys($map)) and the parameters (array_values($map)). As for the $assignment variable: that really should be an argument that the user passes to your function, so change its signature to:

function inserTest(PDO $db, array $assignment)
{}

In the long run, you'll probably want to turn this function into a method, and define $map as a property, which you can then use to validate $assignment (check if the keys in the array exist in $this->map and throw exceptions if something doesn't quite add up)

Prepared statements are reusable

Another drawback of the way you're using prepared statements in your function is that you're missing out on one of their biggest advantages: prepared statements can be re-used several times:

$stmt = $pdo->prepare('INSERT INTO foo (field1, field2) VALUES (:foo, :bar)');
$stmt->execute([':foo' => 'value 1', ':bar' => 1]);
$stmt->execute([':foo' => 'value 2', ':bar' => 2);

The code above will add 2 rows to the foo table, using the same prepared statement. Creating such a statement in a function, using it and then returning a boolean will GC (garbage collect) the prepared statement, only to create it a second time after when you call the function again. This isn't ideal, obviously.

It might be more useful to harness the power of the PDOStatement instance that PDO::prepare returns.
This can be especially useful if you're using SELECT queries: returning the PDOStatement instance allows the user to fetch the data in a format he sees fit:

$stmt = $obj->getSelectByIdStmt();
$data = [];
foreach ($ids as $id) {
    $stmt->execute([':id' => $id]);
    $objects = [];
    //or PDO::FETCH_ASSOC, PDO::FETCH_CLASS, ...
    while ($row = $stmt->fetch(PDO::FETCH_OBJ) {
        $objects[] = $row;
    }
    $data[$id] = $objects;
}

This is something that is quite hard to abstract into one, comprehensive function/method...

Other niggles:

Your usage of this (badly) named $array variable doesn't really make sense:

$array = array();
$array['table'] = 'users';
$array['field_names'] = $field_names;
$array['alias'] = $alias;
$array['assignment'] = $assignment;

For starters, initializing a variable to an empty array, and then adding keys one by one is a lot more work (to type) than a simple:

$array = [
    'table'       => 'users',
    'field_names' => $field_names,
    'alias'       => $alias,
    'assignment'  => $assignment,
];

Now if you were to pre-format/stringify array values (like $field_names), you could use vsprintf to create your query, but that's something others who work on the same code will hate you for (they really will, trust me, I've been there).
I've already hinted at the possibility of creating a class, which could be used to store things that are, essentially, immutable (like the table name and the field map). But that's a different matter, for now: just start by leaving out the $array variable. There's also no need to assign the format to $q if you're reassigning $q the next statement. Just write:

$q = sprintf(
    'INSERT INTO `%s` (%s) VALUES (%s)',
    'users',
    implode(', ', $field_names),
    implode(', ', $alias)
);

The backticks around the table name imply you're using MySQL, you might want to add the same backticks around your field names (in case some mug decides to call fields "transaction" or "set" - again, I've been there, it happens):

$q = sprintf(
    'INSERT INTO `%s` (`%s`) VALUES (%s)',
    'users',
    implode('`, `', $field_names),
    implode(', ', $alias)
);