Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have the following class design for my PDO database access class. Please can someone confirm that this is good practice, or suggest improvements if necessary.

Thanks.

Database.php

class db {
    private $conn;

    public function __construct() {
        $this->conn = new PDO(...);
    }

    $conn = new db();
}

Class.php

// require Database.php

class someClass {
    private $conn;

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

    function myFunc($usr, $pwd) { 
        // SQL
    } 
}

Main.php

// require Class.php
$myObj = new someClass($conn);

$myObj->myFunc(PARAMS);
share

migration rejected from stackoverflow.com Feb 21 at 18:46

This question came from our site for professional and enthusiast programmers. Votes, comments, and answers are locked due to the question being closed here, but it may be eligible for editing and reopening on the site where it originated.

closed as off-topic by Jamal Feb 21 at 18:46

If this question can be reworded to fit the rules in the help center, please edit the question.

1  
I think just creating instance of pdo and then pass it to the constructor your someClass is enough. –  PLB Jul 17 '12 at 12:03
    
Seconded, though if you were to do this, extending the second class off the first, or just combining them, would be preferable. Especially if one is necessary for the other. But don't use classes just to be using classes. –  mseancole Jul 19 '12 at 14:37
    
This question appears to be off-topic because it is primarily a design review and not a code review. –  Jamal Feb 21 at 18:46

2 Answers 2

Your db class is not valid

You can't have a variable assignment in a class outside a function. $conn = new db(); is not a valid syntax unless you put it in a function. Also recreating the same class inside that class does not make sense.

Is it providing any benefits

Is your database class adding any new functionality to the PDO class? If not, there is no point in doing what you do unless you want to create a wrapper around PDO.

share

Might be easier to just extend the PDO class to do what you want.

i.e.

class db extends \PDO {}

class foo {
    private $conn;

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

    // ALL OTHER NEW METHODS
}

require Database.php
$conn = new db(PARAMS);

require Foo.php
$foo = new foo($conn);
$foo->someMethod();

This is assuming that you will have functionality OVER-AND-ABOVE what the PDO class does. If you DO NOT, there is no reason to extend the PDO class.

share