patternphpMinor
PDO Insert Method
Viewed 0 times
insertmethodpdo
Problem
I'm currently creating a class that will have a bunch of my most common database functions. This is the first method I've made, I would appreciate any feedback, thanks!
public static function insert($table, $params) {
// Define initial query
$query = "INSERT INTO `{$table}` (";
// Format rows to insert
foreach($params as $key => $value) {
$fields .= $key . ",";
$bindParams .= ":" . $key . ",";
$queryParams[':' . $key] = $value;
}
// Add rows and bind params to query
$query .= rtrim($fields, ',') . ") VALUES(" . rtrim($bindParams, ',') . ")";
// Prepare Query
$preparedQuery = self::getInstance()->getConnection()->prepare($query);
// Attempt to execute Query
if(!$preparedQuery->execute($queryParams)) {
return false;
}
// If everything passes, return true
return true;
}Solution
The code, for what it is, looks fine. You're using prepared statements, which is a good thing, and the code comments help to understand what it does.
The real problem that I see is more of a design issue. Dynamically building SQL queries should be avoided unless it is absolutely necessary (such cases are pretty rare). They are error-prone, very difficult to debug, not practically testable, and their database execution performance is usually sub-par.
A better approach is to make a method/function for each kind of query you need. Granted, that does make it so you have to write a lot more code, but in the end it will make the code base a lot more maintainable, as well as testable, and your database server will thank you for it.
I would suggest reading The Codeless Code: Case 213 "The Imperfect Mirror" which illustrates how database query related code should be designed.
The real problem that I see is more of a design issue. Dynamically building SQL queries should be avoided unless it is absolutely necessary (such cases are pretty rare). They are error-prone, very difficult to debug, not practically testable, and their database execution performance is usually sub-par.
A better approach is to make a method/function for each kind of query you need. Granted, that does make it so you have to write a lot more code, but in the end it will make the code base a lot more maintainable, as well as testable, and your database server will thank you for it.
I would suggest reading The Codeless Code: Case 213 "The Imperfect Mirror" which illustrates how database query related code should be designed.
Context
StackExchange Code Review Q#151364, answer score: 4
Revisions (0)
No revisions yet.