Passing PDO object into class - PHP Fatal error: Call to a member function execute() on a non-object

Question

I am refactoring a (procedural) PHP Library I wrote a while back into a lightweight OOP framework. I'm getting caught up with trying to pass a PDO object to be used in a class. Here's what I've got so far.

Config.php

<?php

class Config {
    // Database Variables
    private $db_type;
    private $db_host;
    private $db_user;
    private $db_pass;
    private $db_name;
    private $db_path; // for sqlite database path
    private $db_char; // charset

    // Site Variables
    private $s_protocol;
    private $s_subdomain;
    private $s_domain;
    private $s_tld;
    private $s_dir;
    private $s_name;
    private $s_description;
    private $s_path;
    private $s_visibility;
    private $s_pipe;
    private $s_apps;
    private $s_hooks;
    private $s_blocks;
    private $s_assets;

    // User Default
    private $u_groupid;

    public function __construct($config) {
        $this->set($config);
    }

    public function set($config) {
        if (!empty($config) && is_array($config)) {
            foreach ($config as $k => $v) {
                if (property_exists(get_class($this), $k)) {
                    $this->$k = $v;
                }
            }
            return true;
        }
        else { return false; }
    }

    public function get($config) {
        if (!empty($config)) {
            return $this->$config;
        }
    }

    public function domain() {
        return $this->get('s_protocol') .'://'. $this->get('s_domain') . $this->get('s_tld') .'/'. $this->get('s_dir');
    }
}
?>

Database.php

<?php

class Database extends PDO {
    private $config;

    public function __construct($config) {
        $this->config = $config;
        switch($this->config->get('db_type')) {
            case 'mysql':
            case 'pgsql':
                try {
                    return new PDO(
                                $this->config->get('db_type') .':dbname='. $this->config->get('db_name') .';host='. $this->config->get('db_host'),
                                $this->config->get('db_user'),
                                $this->config->get('db_pass')
                    );
                }
                catch(PDOException $e) {
                    die($e->getMessage());
                }
                break;
            case 'sqlite':
                try {
                    return new PDO($this->config->get('db_type') .':'. $this->config->get('db_path'));
                }
                catch(PDOException $e) {
                    die($e->getMessage());
                }
                break;
            case 'firebird':
                try {
                    return new PDO(
                                $this->config->get('db_type') .':dbname='. $this->config->get('db_host') .':'. $this->config->get('db_path'),
                                $this->config->get('db_user'),
                                $this->config->get('db_pass')
                    );
                }
                catch(PDOException $e) {
                    die($e->getMessage());
                }
                break;
            case 'informix':
                try {
                    return new PDO(
                                $this->config->get('db_type') .':DSN='. $this->config->get('db_name'),
                                $this->config->get('db_user'),
                                $this->config->get('db_pass')
                    );
                }
                catch(PDOException $e) {
                    die($e->getMessage());
                }
                break;
            case 'oracle':
                try {
                    return new PDO(
                                'OCI:dbname='. $this->config->get('db_name') .';charset='. $this->config->get('db_char'),
                                $this->config->get('db_user'),
                                $this->config->get('db_pass')
                    );
                }
                catch(PDOException $e) {
                    die($e->getMessage());
                }
                break;
        }
    }


}
?>

Auth.php

<?php

class Auth {
    // Set Database object
    protected $db;

    // User fields in users table
    private $id;
    private $email;
    private $password;
    private $firstname;
    private $lastname;
    private $displayname;
    private $groupid;
    private $ip;
    private $created;
    private $updated;
    private $cookie;
    private $sessionid;
    private $lastlogin;
    private $token;
    private $active;

    public function __construct($dbh) {
        $this->db = $dbh;
    }

    public function add($params) {
        $sql = '
            INSERT INTO
                `users` (
        ';
        $cols = array_keys($params);
        $col_string = implode(', ', $cols);
        $sql .= $col_string .'
                )
            VALUES (
        ';
        array_walk($cols, function(&$v, $k) { $v = ':'. $v; });
        $col_string = implode(', ', $cols);

        $sql .= $col_string .'
                )
        ';
        $stmt = $this->db->prepare($sql);
        $stmt->execute($params);

    }

    public function remove($params) {

    }

    public function update($params) {

    }

    public function get($params) {

    }

    protected function set($params) {
        if (!empty($params) && is_array($params)) {
            foreach ($params as $k => $v) {
                if (property_exists(get_class($this), $k)) {
                    $this->$k = $v;
                }
            }
            return true;
        }
        else { return false; }
    }
}

?>

init.php

<?php
session_start();
$params = array(
                'db_type' => 'mysql',
                'db_host' => '127.0.0.1',
                'db_user' => 'user',
                'db_pass' => 'password',
                'db_name' => 'database',
                'u_groupid' => 4
            );
require_once('Config.php');         $c = new Config($params);
require_once('Database.php');       $db = new Database($c);
require_once('Auth.php');           $u = new Auth($db);

$user = array(
    'email' => '[email protected]',
    'password' => md5('password'),
    'firstname' => 'Jeff',
    'lastname' => 'Wilson',
    'displayname' => 'Jeff Wilson',
    'groupid' => $c->get('u_groupid'),
    'ip' => $_SERVER['REMOTE_ADDR'],
    'created' => date('Y-m-d H:i:s'),
    'sessionid' => session_id(),
    'active' => 1,
);
$u->add($user);
?>

PHP Fatal error: Call to a member function execute() on a non-object in Auth.php on line 46

This is line 46: $stmt->execute($params);

As far as I know I'm passing the PDO object to the Auth class correctly. It shouldn't say it's a non-object. Can anyone else see what's wrong here?

Looks like $this->db->prepare($sql) failed and the pdo instance isn't set to ERRMODE_EXCEPTION -> $stmt "is" FALSE. — VolkerK
– VolkerK, Commented Nov 29, 2015 at 7:19
@VolkerK - I have added these settings and the page still throws a 500 Internal Server Error and checking the http log it still says the same thing - PHP Fatal error: Call to a member function execute() on a non-object in Auth.php on line 46. — Armin
– Armin, Commented Nov 29, 2015 at 7:23

Alex Ivey · Accepted Answer · 2015-11-29 07:58:34Z

2

The constructor in the Database class is returning a value (a PDO object). The __construct() function should not explicitly return a value. Since your Database class extends PDO, call the parent constructor instead:

parent::__construct(
    $this->config->get('db_type') .':dbname='. $this->config->get('db_name') .';host='. $this->config->get('db_host'),
    $this->config->get('db_user'),
    $this->config->get('db_pass')
);

answered Nov 29, 2015 at 7:58

Alex Ivey

463 bronze badges

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

Comments

pavlovich · Accepted Answer · 2015-11-29 07:47:56Z

Add this method to your Database class

   public function getConnection() {
        return new PDO(
            $this->config->get('db_type') .':dbname='. $this->config->get('db_name') .';host='. $this->config->get('db_host'),
            $this->config->get('db_user'),
            $this->config->get('db_pass')
        );
    }

call prepare statement like this:

$conn = $this->db->getConnection();
$stmt = $conn->prepare($sql);

Your $conn has to be PDO object in this case

The main problem here is while $db = new Database($c); seems to be fine at the first glance, calling $db->prepare is not good because $db is instance of Database, but has to be instance of PDO

One of the ways to improve a bit connection handling would be: In your Database class to have a private $conn for connection

class Database extends PDO {
    private $config;
    private $conn;

     public function __construct($config) {
        $this->config = $config;
        switch($this->config->get('db_type')) {
            case 'mysql':
            case 'pgsql':
               try {
                    $this->conn = new PDO(
                        $this->config->get('db_type') . ':dbname=' . $this->config->get('db_name') . ';host=' . $this->config->get('db_host'),
                        $this->config->get('db_user'),
                        $this->config->get('db_pass')
                    );
                } catch(PDOException $e) {
                    die($e->getMessage());
                }

            break;

         // ...

        }
    }

THEN in the same class new method to return connection:

public function getConnection() {
        return $this->conn;
    }

AND FINALLY call it:

$this->db->getConnection()->prepare($sql)

Community · Accepted Answer · 2017-05-23 10:27:11Z

1

Unless the pdo instance is set explicitly to use exceptions for error reporting you have to check the return value of PDO::prepare

$stmt = $this->db->prepare($sql);
if ( !$stmt ) {
    // prepare failed
    // the array returned by $this->db->errorinfo() most likely contains more info about the error
    // don't send it unconditionally,directly to the browser, see https://www.owasp.org/index.php/Top_10_2013-A6-Sensitive_Data_Exposure

}
else {
    $result = $stmt->execute($params);
    if ( !$result ) {
        // not ok
    }
    else {
        // ok
    }
}

edit: I can leave out the first part of my answer, since Alex Ivey already wrote it. ;-)

Just image behind the scene php is doing something like

function __internal_newDatabase() {
    // just image the parser/compiler creates this function
    // from your class defintion
    $instance = array(
        'properties'=>array('config'=>null),
        'methods'=>array('beginTransaction'=>PDO::beginTransaction, 'prepare'=>PDO::prepare, ...)
    );  
    // some magic function that calls a method and "replaces" $this by the second parameter
    invoke(Database::__construct, $instance);
    return $instance;
}

and when your script contains new Database instead it calls __internal_newDatabase(). That's (ooooversimplified) what happens and therefore you can not just "change" the instance within your constructor "method" by returning a different instance. Your constructor is supposed to make this instance fly (or bail out by throwing an exception).
Your class Database is derived from PDO, i.e. it is supposed to behave as a PDO. In other languages that implies that the/a base class' constructor must be called. PHP doesn't enforce that. But in this case leaves your Database instance unusable. You have to explcitily call the parent's constructor as Alex's answer shows.

But there are other issues with this class. (First a confession: I'm biased as can be against class Database. In virtually all in all cases it's just wrong and therfore automagically raises a red flag for me)
Foremost: Given its name and the way you use it, it's superfluous. It's just a configuration detail not a class derived from PDO. More likely it would be a factory and/or something within an IoC container (if you use that).
On the other hand it might not be just a configuration detail but might (and probably will) cause different implementions for different databases. PDO is not a database abstraction, just a unified access layer.
Your class Auth.php doesn't care about the concrete sql dialect used - and this particular query will most likely work for all database systems supported by PDO. But there will sooner or later be queries that have to be customized for the different RDBMSs. And then your base class would probably be called something like DB_Adapter and there would be MySQL_Adapter extends DB_Adapter and so on....

edited May 23, 2017 at 10:27

CommunityBot

11 silver badge

answered Nov 29, 2015 at 7:24

VolkerK

96.3k20 gold badges169 silver badges232 bronze badges

4 Comments

Armin Over a year ago

When I put this in it does not give me a 500 Internal Server Error - but checking the http logs it throws this: PHP Notice: Undefined property: Database::$errorinfo

VolkerK Over a year ago

errorinfo isn't a property but a method returning an array. see docs.php.net/pdo.errorinfo

Armin Over a year ago

Ah ok right! My apologies. This is what is returned in the http logs - PHP Warning: PDO::errorInfo(): SQLSTATE[00000]: No error: PDO constructor was not called

VolkerK Over a year ago

Oh, now the question gets interesting ;-) ...because I only skimmed over the code. Your class Database extends PDO and when you call $this->prepare() it actually calls PDO::prepare but this instance hasn't been initialized properly (...because you can instantiate derived classes without calling any constructor of the base class; one of my less liked "features" of php). I'll edit the answer, but the summary is: Database::__construct isn't supposed to do something like return new PDO(..., it is already working on the instance that is to be initialized.

Collectives™ on Stack Overflow

Passing PDO object into class - PHP Fatal error: Call to a member function execute() on a non-object

3 Answers 3

Comments

Comments

4 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

Comments

4 Comments

Your Answer

Sign up or log in

Post as a guest

Related