2
\$\begingroup\$

Is there a better way to write this?

$query = $db->getConnection()->prepare("INSERT INTO `users` (first_name, last_name, email, username, password, unique_id, date_joined) VALUES(:fname, :lname, :email, :username, :pass, :uniqueid, :date_joined)");
$query->bindParam(':fname', $first_name, PDO::PARAM_STR);
$query->bindParam(':lname', $last_name, PDO::PARAM_STR);
$query->bindParam(':email', $email, PDO::PARAM_STR);
$query->bindParam(':username', $username, PDO::PARAM_STR);
$query->bindParam(':pass', $passHash, PDO::PARAM_STR);
$query->bindParam(':uniqueid', $emailHash, PDO::PARAM_STR);
$query->bindParam(':date_joined', $date, PDO::PARAM_STR);
$query->execute();
\$\endgroup\$
  • \$\begingroup\$ As we all want to make our code more efficient or improve it in one way or another, try to write a title that summarizes what your code does, not what you want to get out of a review. Please see How to get the best value out of Code Review - Asking Questions for guidance on writing good question titles. \$\endgroup\$ – BCdotWEB Jan 29 '16 at 12:16
2
\$\begingroup\$

Since you have PDO::PARAM_STR everywhere, you can just pass all the parameters to the execute:

$query = $db->getConnection()->prepare("INSERT INTO `users` (first_name, last_name, email, username, password, unique_id, date_joined) VALUES(:fname, :lname, :email, :username, :pass, :uniqueid, :date_joined)");
$query->execute(array(
    ':fname' => $first_name,
    ':lname' => $last_name,
    ':email' => $email,
    ':username' => $username,
    ':pass' => $passHash,
    ':uniqueid' => $emailHash,
    ':date_joined' => $date
));

What about if one wasn't a string? Would the "best" practice be to follow this and just bindParam as usual on the variable that isn't PDO::PARAM_STR?

You can have a custom version of the execute which could work like this:

myCustomExecute($query, array(
    ':fname' => [$first_name, PDO::PARAM_STR]
    ':lname' => [$last_name, PDO::PARAM_STR],
    ':age' => [$age, PDO::PARAM_INT]
));

Inside the myCustomExecute you just loop over the array and do bindValue for each array item (note, it is better to use bindValue instead of bindParam, see for example, this).

But I feel like you want something bigger than this to avoid writing a boilerplate code. In this case your are looking for an ORM.

The practical solution is to search for existing solution and choose the one you like.

Less practical way would be to write something simple around PDO.

The final syntax would be something like this:

// ActiveRecord pattern
$model = new Person();
$person->first_name = 'John';
$person->age = 22;
$person->save();

// OR, in the case of DataMapper pattern, you have separate
// object for your model and another one which handles
// the database operation
$table = new PersonMapper();
$person = new Person();
$person->first_name = 'John';
$person->age = 22;
$table->save($person);

The record class or mapper class usually have some kind of metadata (you define it manually or they get this metadata directly from the database), so they know the type of each filed, as well as some other limitations (like uniqueness checks). And usually you are able to specify some own validation rules to check before the data is saved to the database.

This removes a lot of the boiler plate, because with plain PDO you have to repeat your fields for times - twice in the SQL (first_name -> :fname) and then twice in the bindValue or execute calls.

\$\endgroup\$
  • \$\begingroup\$ What about if one wasn't a string? Would the "best" practice be to follow this and just bindParam as usual on the variable that isn't PDO::PARAM_STR? \$\endgroup\$ – Joe Scotto Feb 6 '16 at 8:07
  • \$\begingroup\$ @JoeScotto I updated the answer and I hope that my guess about what you actually want to have is correct. \$\endgroup\$ – Boris Serebrov Feb 6 '16 at 11:17

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.