Implementing PDO, code correct so far?

Question

I have decided to go PDO and have started implementing it on one of my webprojects. I have limited programing skills but can usually pull it off with some tutorials and forums.

Here is my code this far and it works fine but how is the code correct regarding the picking up errors, syntax, order and begintransaction etc.? Have I missunderstood anything? Is anything unneccesary?

connect.php

 <?php
try {
    $DBH = new PDO("mysql:host=$host;dbname=$database", $username, $password, array(
        PDO::ATTR_PERSISTENT => true
    ));
    $DBH -> exec("set names utf8");
}
catch(PDOException $e) {
    echo $e->getMessage();
}
?>

query.php

try {  
        $DBH->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);

        $DBH->beginTransaction();   

        $STH = $DBH->prepare('SELECT id from users where uid = :uid');
        $STH->setFetchMode(PDO::FETCH_ASSOC); 
        $STH->bindParam(':uid', $uid); // $uid value is set
        $STH->execute();

        $uid_in_db = $STH->rowCount();
        if($uid_in_db==0){ //=new user, insert info in db.
            $STH = $DBH->prepare("INSERT INTO USERS (uid,namn) VALUES (:uid, :name)");
            $STH->bindParam(':uid', $uid);
            $STH->bindParam(':namn', $_POST['namn']); // a value posted form user input
            $STH->execute();
        }

        $DBH->commit(); // 

    } catch (Exception $e) {
        $DBH->rollBack();
        echo "Fel: " . $e->getMessage();
    }
}else{
    $error=1;
}

I don't see any need for transactions. You only have 1 insert. Do you plan to add more write queries? Note that by beginning a transaction you don't issue any implicit locks on the tables.
Thanks, I changed that since now. What do you mean by mean with don't issue any implicit locks on the tables? thanks!

Kendall Hopkins · Accepted Answer · 2011-04-18 14:38:20Z

up vote 2 down vote accepted

Instead of checking for a UID collision before an insert, I would put a unique key on that column, and then write fallback code for the case (very very unlikely) when two collide. That way you can simplify your logic and reduce the number of queries to one instead of two. You also don't need a transaction if you implement this change, since there is only 1 query.

//you can remove the outer try/catch since only the execute() should possibly fail
$DBH->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); //<~ put this in the initialize of the db connection
$STH = $DBH->prepare("INSERT INTO USERS (uid,namn) VALUES (:uid, :name)");
$STH->bindParam(':uid', $uid);
$STH->bindParam(':namn', $_POST['namn']); // a value posted form user input
try {
    $STH->execute();
} catch( PDOException $e ) {
    //deal with collision 
}

edited Apr 18 '11 at 14:38

answered Apr 18 '11 at 14:12

Kendall Hopkins
1363

	1.I put the errormode in under `$DBH -> exec("set names utf8");` is that OK? 1b.what is the differende from `catch(PDOException $e)`and `catch(Exception $e)`? 2. Ichanged so that the try is only around the `$STH->execute();`is that something I can always follow? 3.I did not quite understand the uid as key and the fallback case. But I would really like to slim the script. Though I might need to specify that the uid is the uid from FBAPI that is set when a user login to the site and the script should see if the user is in the db or show the form to add info. so the uid will in most cases exist. – Joseph Apr 18 '11 at 14:35
	@Joseph PDOException is more correct, since it would let any other exception (which would be more serious) to be emitted (I updated my example). If you put a unique key on column in the database, the database will reject any duplicate uid entires and cause `$STH->execute()` to raise an exception. – Kendall Hopkins Apr 18 '11 at 14:43
	Ok now I understand but do you think it is a good solution since as I described the id will in most cases be a duplicate (for every pageload by a returning user) and thus be an error. Also question 1 in comment above, is the catch error line put correctly and should I remove it when in production? Thanks! – Joseph Apr 18 '11 at 14:58
	@Joseph Taking that into consideration, I might actually lean towards you original solution. Since the normal case would be to only run 1 query. I was under the impression that you were creating a user entity, not a session. I wouldn't consider it good practice to use an insert to check if a row already exists. – Kendall Hopkins Apr 18 '11 at 15:08
	Ok thanks, yes I was a bit unclear. Can you please look at the other questions I gave in the comments above regarning your other suggestions. thanks – Joseph Apr 18 '11 at 15:18

TaylorOtwell · Answer 2 · 2011-04-18 14:15:15Z

up vote 1 down vote

You may want to check out the disadvantages of using persistent connections with PDO. You may not really need to use that option.

What are the disadvantages of using persistent connection in PDO?

answered Apr 18 '11 at 14:15

TaylorOtwell
1234

exactly the type of feedback i needed. on first reading in the manual it seemed good but after further research it oes not seem to fit my site. I removed it – Joseph Apr 18 '11 at 14:38

asked	2 years ago
viewed	561 times
active	2 years ago

Implementing PDO, code correct so far?

migrated from stackoverflow.com Apr 18 '11 at 14:25

2 Answers

Your Answer

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

Implementing PDO, code correct so far?

migrated from stackoverflow.com Apr 18 '11 at 14:25

2 Answers

Your Answer

Sign up or login

Post as a guest

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

Related