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 small Class which handles user's information. (name, surname, login, hash of password). Also this class can activate, register and login the user. This means I should connect to DB. Is it normal? I'm using Pear MDB2 to connect to DB.

Does it mean that every instance of User will have MDB2 object initialised?

class User {
    private $login;
    private $hash;
    private $name;

    /* some getters and setters */
    //
    // public function get_name() { return $this->name }
    // ..
    //

    private $connect; // MDB2 connection
    private $dsn = ""; // connection config

    public function exists() { // return true if User exists
        require ('MDB2.php');
        $this->connect = MDB2::connect($this->dsn);
        // check
    }
}

Should I make $connect and $dsn variables static? Or may be I should use some pattern for this idea?

Update: (kind of decision)

I store database connection in static variable:

class DataBase {
    // connections options
    static public $connect = NULL;      // MDB2 connection variable
    static public $dsn = "pgsql://user:pass@IP/database";
    static public $connected = false;

    static function reportError() {
        if (PEAR::isError(self::$connect)) {
            $res["success"]=false;
            $res["errors"][] = self::$connect->getMessage();
            echo json_encode($res);
            exit;
        }
    } //: reportError

} //: Class DataBase

So If I want to connect to DB I write:

// connect only if it's not connected
if (!DataBase::$connected) {
    require_once('MDB2.php'); // Pear MDB2 class
    DataBase::$connect = MDB2::factory(DataBase::$dsn); // connection
    DataBase::$connected = true; // Flag that we're already connected
}

It's normal? Or I should make it another way?

share|improve this question
 
What is MDB2.php doing? –  Chris May 2 '11 at 17:58
 
MDB2.php is Pear MDB2 class. It helps to connect to database of any type (mysql, pqsql, sqlite, etc). To change database it requires only changing of one variable ($dsn) –  Innuendo May 2 '11 at 19:16
add comment

1 Answer

Also this class can activate, register and login the user. This means I should connect to DB. Is it normal?

It's up to you and your requirements. Personally I am trying to make all domain specific objects simple as possible and use it only as Value Object. So, in this case all actions such as checking for existance, creation, activation, registration etc should go to another class, for example, UserManager or similar. And in this class I will hold reference to DB and doing all SQL-queries.

Should I make $connect and $dsn variables static?

Again, it's depends from your requirements. But from my point of view -- no, you shouldn't. When you doing this you cannot have two different connections for MySQL and PostgreSQL, for example.

In any case, I suggest to change public modifier to private (or protected) for $connect, $dsn and $connected memebers.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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