PDO connection / prep and execute in their own functions

Question

I'm currently changing some legacy code over to PDO. The below code works, but not 100%. I would like to know if it's best practice or if anything can be done better.

//Database Array
$config['db'] = array(
    'host' => 'localhost',
    'username' => 'root',
    'password' => 'root',
    'dbname' => 'local');

//New PDO
$db = new PDO('mysql:host=' . $config['db']['host'] . ';dbname=' . $config['db']['dbname'], $config['db']['username'], $config['db']['password']);
$db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
//Check connection is ok
try {
    $db->exec("SET CHARACTER SET utf8");
}
catch (PDOException $ex) {
    print "Error!: " . $ex->getMessage() . "<br/>";
    die();
}

//Update users function
function update($db, $fn, $ln, $email, $offers, $vlue, $response)
{
    $stmt = $db->prepare("insert into local (fName_765, lName_765, email_765, signup_765) values (:fname, :lname, :email, :signup, NOW())");
    $stmt->bindParam(':fname', $fn, PDO::PARAM_STR);
    $stmt->bindParam(':lname', $ln, PDO::PARAM_STR);
    $stmt->bindParam(':email', $email, PDO::PARAM_STR);
    $stmt->bindParam(':signup', $offers, PDO::PARAM_STR);
    try {
        $stmt->execute();
        print $db->lastInsertId(); //show ID
        return true;
    }
    catch (PDOException $e) {
        print "Error!: " . $e->getMessage() . "<br/>"; // show error
        return false;
    }
}

//Test Attributes
$fn = 'test';
$ln = 'test';
$email = 'tesst@test,com';
$offers = '1';

if (!update($db, $fn, $ln, $email, $offers, $vlue, $response)) {
    echo "no update there is a slight problem";
} else {
    echo "it seemed to work";
}

Midified version:

$functionError = array();

//Database Array
$config['db'] = array(
    'host' => 'localhost',
    'username' => 'root',
    'password' => 'root',
    'dbname' => 'root');

//Check connection is ok
try {
    $db = new PDO('mysql:host=' . $config['db']['host'] . ';dbname=' . $config['db']['dbname'], $config['db']['username'], $config['db']['password']);
    $db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
}
catch (PDOException $e) {
    $functionError[] = "<p>{DB} - Error!: " . $e->getMessage() . "</p>";
    errorhandler ($functionError);
    die ();
}

//Error Handler
function errorhandler ($functionError) {
    print_r ($functionError);
    echo "Sorry we are experiencing issues we are aware of the problem";

}

//Update users function
function update($db, $fn, $ln, $email, $offers, $vlue, $response)
{
    $stmt = $db->prepare("insert into kkt (fName_765, lName_765, email_765, signup_765, kkt_resp_765, kkt_respSate_765, stamp_765) values (:fname, :lname, :email, :signup, :kkt_rsp, :kkt_respState, NOW())");

    $parameters = array(
        ':fname' => $fn,
        ':lname' => $ln,
        ':email' => $email,
        ':signup' => $offers,
        ':kkt_rsp' => $vlue,
        ':kkt_respState' => $response);

    try {
        $stmt->execute($parameters);
        return true;
    }
    catch (PDOException $ex) {
        $functionError[] = "<p>{INSERT} - Error!: " . $ex->getMessage() . "</p>";
        errorhandler ($functionError);
        return false;
    }
} 

//Test Attributes
$fn = 'test';
$ln = 'test';
$email = 'tesst@test,com';
$offers = '1';
$vlue = 'value';
$response = 'resp';

if (update($db, $fn, $ln, $email, $offers, $vlue, $response)) {
    echo "it seemed to work";
    print $db->lastInsertId();
    header('Location: thank-you.php');
    exit;
}

Answer 1

Here are a few things I noticed.

1. If you are going to catch a possible exception when you connect to your MySQL DB you want to include the PDO instantiation in the try block.

   try {  
       $db = new PDO('mysql:host=' . $config['db']['host'] . ';dbname=' . $config['db'}['dbname'], $config['db']['username'], $config['db']['password']);
       $db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
       $db->exec("SET CHARACTER SET utf8");
   }  catch(PDOException $e) {  
       print "Error!: " . $ex->getMessage() . "<br/>";
       die(); 
   }
   

2. The arguments $vlue and $response don't do anything in your update function. You probably want to get rid of those.

3. You can use a quicker syntax for binding parameters by defining an array that holds the values and passing it to PDOStatement::execute(). This has the benefit of less function calls. Doing it this way will default all parameter value datatypes to be PDO::PARAM_STR, which matches what you are doing currently.

   $parameters = array(
       ':fname'  => $fn,
       ':lname'  => $ln,
       ':email'  => $email,
       ':signup' => $offers
   );
   $stmt->execute($parameters);
   

4. The point of exceptions is to be able to handle, on your own terms, exceptional events. If a PDO exception occurs in your update function, more than likely it won't/shouldn't know how to handle it. It makes more sense for the code that calls update to take care of that situation. So I would suggest moving the try/catch block to wherever the update function is called.

      try {
          update($db, $fn, $ln, $email, $offers, $vlue, $response)
      } catch (PDOException $e) {
          echo "no update there is a slight problem " . $e->getMessage();
      }