##Is it good practice to insert data into a table this way?
Is it good practice to insert data into a table this way?
###Prepared statements are reusable
Prepared statements are reusable
##Is it good practice to insert data into a table this way?
###Prepared statements are reusable
Is it good practice to insert data into a table this way?
Prepared statements are reusable
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)
);
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)
);
###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...
###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...