1

So i've been having some trouble converting my application from the old mysql syntax to PDO.

Here's what I've tried so far, with the old mysql lines commented out.

db_connect.php
<?php

class DB_Connect {

    // constructor
    function __construct() {

    }

    // destructor
    function __destruct() {
        // $this->close();
    }

    // Connecting to database
    public function connect() {
        require_once 'config.php';
        // connecting to mysql
        try {
        $pdo = new PDO('mysql:host=localhost;dbname=gcm', DB_USER, DB_PASSWORD);
        }
        catch (PDOException $e) {
            $output = 'Unable to connect to database server.' .
            $e->getMessage();
            exit();
        }

       // $con = mysql_connect(DB_HOST, DB_USER, DB_PASSWORD);
        // selecting database
        //mysql_select_db(DB_DATABASE);

        // return database handler
        return $pdo;
    }

    // Closing database connection
    public function close() {
        $pdo = null;
    }

} 
?>

And here's my db_functions class where all of the CRUD operations happen, in which I use the db_connect class to return a connection I can use 

db_functions.php
<?php

class DB_Functions {

private $db;

// constructor
function __construct() {
    include_once './db_connect.php';
    // connecting to database
    $this->db = new DB_Connect();
    $this->db->connect();
}

// destructor
function __destruct() {

}

public function storeUser($name, $email, $gcm_regid) {
    //insert user into database
    try {
        $sql = "INSERT INTO gcm_users(name, email, gcm_regid, created_at) VALUES('$name', '$email', '$gcm_regid', NOW())";
        $result = $db->query($sql);
        if ($result) {
            // get user details
            $id = $db->lastInsertId(); //last inserted id
            $sql = "SELECT * FROM gcm_users WHERE id = $id";
            $result = $db->query($sql);
            $no_of_rows = $result->fetchColumn();
            // return user details
            if ($no_of_rows > 0) {
                     return $result->fetch(PDO::FETCH_ASSOC);
            } else {
                    return false;
            }
    } else {
        return false;
    }
    }
    catch (PDOException $e) {
        $error = 'Error storing user: ' . $e->getMessage();
    }
}


/*    // insert user into database
    $result = mysql_query("INSERT INTO gcm_users(name, email, gcm_regid, created_at) VALUES('$name', '$email', '$gcm_regid', NOW())");
    // check for successful store
    if ($result) {
        // get user details
        $id = mysql_insert_id(); // last inserted id
        $result = mysql_query("SELECT * FROM gcm_users WHERE id = $id") or die(mysql_error());
        // return user details
        if (mysql_num_rows($result) > 0) {
            return mysql_fetch_array($result);
        } else {
            return false;
        }
    } else {
        return false;
    }
}
   */

//Get user by email or password
public function getUserByEmail($email) {
    try {
    $sql = "SELECT * FROM gcm_users WHERE email = '$email' LIMIT 1";
    $result = $db->query($sql);
    return $result;
    }
    catch (PDOException $e) {
        $error = 'Error fetching user by email: ' . $e->getMessage();
    }
  //  $result = mysql_query("SELECT * FROM gcm_users WHERE email = '$email' LIMIT 1");

}

//Returns all users
public function getAllUsers() {
    try {
    $sql = "select * FROM gcm_users";
    //$result = mysql_query("select * FROM gcm_users");
    $result = $db->query($sql);
    return $result;
    }
    catch (PDOException $e) {
        $error = 'Error getting all users: ' . $e->getMessage();
    }
}



//Check if user exists
public function isUserExisted($email) {
    try {
        $sql = "SELECT email from gcm_users WHERE email = '$email'";
        $result = $db->query($sql);
        $no_of_rows = $result->fetchColumn();


    //$result = mysql_query("SELECT email from gcm_users WHERE email = '$email'");
    //$no_of_rows = mysql_num_rows($result);
    if ($no_of_rows > 0) {
        // user existed
        return true;
    } else {
        // user not existed
        return false;
    }
    }
    catch (PDOException $e) {
        $error = 'Error fetching user by email: ' . $e->getMessage();
    }
}

I tested the code using only the db_connect class and it seemed to be connecting to the database okay, but when I started updating the db_functions class I get the error 

Notice: Undefined variable: db in C:\xampp\htdocs\gcm\db_functions.php on line 84

Fatal error: Call to a member function query() on a non-object in      C:\xampp\htdocs\gcm\db_functions.php on line 84

This is line 84

   public function getAllUsers() {
        try {
        $sql = "select * FROM gcm_users";
        //$result = mysql_query("select * FROM gcm_users");
        $result = $db->query($sql); //Line 84
        return $result;

It seems to have something to do with it not recognizing the $db as representing the PDO connection object returned from the connect() method in db_connect. Any way to go about fixing this?

EDIT:

Tried getting rid of db_connect.php all together and creating the PDO connection in db_functions.php

class DB_Functions {

    private $db;

    // constructor
    function __construct() {
        require_once 'config.php';
        // connecting to mysql
        try {
        $this->$db = new PDO('mysql:host=localhost;dbname=gcm', DB_USER, DB_PASSWORD);
        }
        catch (PDOException $e) {
            $output = 'Unable to connect to database server.' .
            $e->getMessage();
            exit();
        }
    }

    // destructor
    function __destruct() {

    }

//Returns all users
public function getAllUsers() {
    try {
    $sql = "select * FROM gcm_users";
    //$result = mysql_query("select * FROM gcm_users");
    $result = $this->$db->query($sql);
    return $result;
    }
    catch (PDOException $e) {
        $error = 'Error getting all users: ' . $e->getMessage();
    }
}

Now getting the error:

Notice: Undefined variable: db in C:\xampp\htdocs\gcm\db_functions.php on line 12

Fatal error: Cannot access empty property in C:\xampp\htdocs\gcm\db_functions.php on line 12

This is line 12:

$this->$db = new PDO('mysql:host=localhost;dbname=gcm', DB_USER, DB_PASSWORD);

I tried using the outline in the first answer listed here :How do I create a connection class with dependency injection and interfaces?

Improve this question
3
  • If you're using PDO, use prepared statements/bind variables Commented Jun 16, 2014 at 21:16
  • $db->query($sql); should be $this->db->query($sql); ` Commented Jun 16, 2014 at 21:17
  • It looks like you're writing your own ORM. Why not use one that's already written and tested like Propel or Doctrine? Commented Jun 16, 2014 at 21:25

1 Answer 1

Reset to default
1

You are not storing the result of $this->db->connect() to any variable you can later use, so your class has no reference to a DB connection whatsoever from the start.

You then also do not properly refer to (what is supposed to be) the DB connection at $this->db instead only calling $db->*.

You have a sort of an ill-conceived approach of using a singleton, but I would suggest simply using dependency injection. So perhaps drop the use of DB_Connect class and simply inject a PDO object directly into your DB_Function class (which should rightfully probably be called a user class).

class DB_Functions {

    // note I changed this to protected in case you actually decide
    // to change class name to something meaningful like "user"
    // and want to ever subclass it        
    protected $db;

    // constructor
    function __construct(PDO $pdo) {
        $this->db = $pdo;
    }

    // rest of your class
    // make sure you replace $db->* references with $this->db->*
}

This does a few things. It will actually fail instantiation of the DB_Functions object if a properly instantiated PDO object is not passed in. You are "injecting" your dependency (having a valid PDO object) into this class. Without it, object instantiation fails, which is probably a desirable behavior, as there is no use in having that object instantiated if there is no valid DB connection.

This also helps decouple your "user" logic from DB instantiation logic. After all, why should this user class need to know how to go about instantiating a PDO object. Some other portion of code should handle setting up the PDO connection and pass it in, such that at some point down the line, when you want to stop using DB_Connect in favor of some other PDO provider, you would not then need to change this class and others like it to refer to some new PDO provider class.

Improve this answer
Sign up to request clarification or add additional context in comments.

4 Comments

@ronpaul Why not just pass in the PDO object to class constructor? You have now even more tightly coupled your class functionality with the logic to instantiate the DB. Your actual problem however is that you are referring to $this->$db a variable variable in several places instead of $this->db.
@ronpaul You also didn't really follow the example you stated at all, which showed a concrete MySQL class from which an object wold be instantiate and then passed to the Test class at instantiation. That was a dependency injection example, your current code is not because your dependency is actually constructed inside your class.
Mike, my problem with that is that it seems like it would get repetitive to create a new PDO connection in every class before I call db_functions, but I will heed your advice and try to pass the PDO into the constructor.
@ronpaul That is the point. You DON'T instantiate the PDO object inside every class that needs it. You instantiate it once somewhere separately in your code, then "inject" it into the classes that need it. It also sounds like you may have a bit of a bad class design scheme if there are others classes which are calling DB_Functions, which seems to contain logic very specific to the concept of a "user". However, if there are multiple classes that use DB_Functions you can in turn instantiate one object of that class (which has nested PDO object) and "inject" it into the classes that need it.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.