HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

PDO Insert Method

Submitted by: @import:stackexchange-codereview··
0
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.

Context

StackExchange Code Review Q#151364, answer score: 4

Revisions (0)

No revisions yet.