PHP Best practice

[extjac]

hello, i was wondering if you could tell me what would you diffent/better on the following code. I am trying to become a better developer and i would like to learn what the best PHP pratice is. I remove the comments to make it a bit cleaner

//THIS IS THE CLASS I USE TO CONNECT TO THE DB - I USE PDO

class db 
{
    private static $instance = NULL;

    private function __construct() 
    {
    }
    public static function getInstance() 
    {
        if (!self::$instance)
            {
            self::$instance = new PDO("mysql:host=localhost;dbname=DB", '', '');;
            self::$instance-> setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
            }
    return self::$instance;
    }
    private function __clone()
    {
    }

} /*** end of class ***/






//ONE CLASE AS EXPAMPLE
class Contact 
{
    function __construct()
    {   
    }

    //LOGIN FUNCTION
    public  function login()
    {
            $sql  = "   SELECT  * 
                        FROM    contacts 
                        WHERE   username = :username AND password = :password 
                        LIMIT 1;    
                    ";        
       try
        {
            $rs = db::getInstance()->prepare($sql);
            $rs->bindParam(':username'  , $this->username);
            $rs->bindParam(':password'  , $this->password);
            $rs->execute();
             if($rs->rowCount() > 0 )
             {
                     //USER EXIST
                    $result = $rs->fetch(PDO::FETCH_ASSOC);
                    $this->login = 1;
                    @session_start();
                    $_SESSION["login"]= $this->login ;
                    foreach($result as $key => $value) 
                    { 
                        $_SESSION[$key]     = $value; 
                    }   
                    header('Location: home/');
             }else{
                 /* user not exist*/
                 $this->error = 'Username/Password not valid. Try again';
                 return false;
             }
        } catch (PDOException  $e)  { 
            print $e->getMessage();
        }
    }




    //GET ONE RECOROD
    public  function getRecord($var)
    {
       try
        {
            $sql  = "   
                        SELECT      `contacts`.*
                        FROM        `contacts` 
                        WHERE       `contacts`.`contact_id` = :id 
                        LIMIT 1;
                    ";        
            $rs = db::getInstance()->prepare($sql);
            $rs->bindParam(':id'    , $var);
            $rs->execute();
            if($rs->rowCount() > 0 )
            {
                $result = $rs->fetch(PDO::FETCH_ASSOC);
                    foreach($result as $key => $value) 
                    {
                        $this->$key = $value;
                    }
            }
        }  
        catch (PDOException  $e)    
        {
            print $e->getMessage();
        }
    }

    // GET ALL RECORDS
    public  function getRecords()
    {
            $sql  = "
                    SELECT  `contacts`.*
                    FROM    `contacts`          
                    ";      
       try
        {
            $sth = db::getInstance()->prepare($sql);
            $sth->execute();
            while($row = $sth->fetch(PDO::FETCH_ASSOC))
            {
                $result[] = $row;
            }
        return $result;
        }
       catch (PDOException  $e)
        {
            print $e->getMessage();
        }
    }

    // INSERT A NEW CONTACT
    public function insert()
    {       
            $sql  = "INSERT INTO contacts 
                                    (username   , password  ,name) 
                            VALUES 
                                    (:username  ,:password  ,:name) ;
                    ";
       try
        {
            $sth = db::getInstance()->prepare($sql);
            $sth->bindParam(':username'         , $this->username   );
            $sth->bindParam(':password'         , $this->password   );
            $sth->bindParam(':name'             , $this->name       );
            $sth->execute();
        return db::getInstance()->lastInsertId();
        }
       catch (PDOException  $e)
        {
            print $e->getMessage();
        }
    }


    //UPDATE CONTACTS
    public function save($var)
    {       
        $sql  = " 
                        UPDATE contacts SET

                                     username       = :username
                                    ,password       = :password
                                    ,name           = :name

                        WHERE contact_id = :id
             ";
        try
        {
            $sth = db::getInstance()->prepare($sql);
            $sth->bindParam(':id'           , $var, PDO::PARAM_INT);
            $sth->bindParam(':username'     , $this->username);
            $sth->bindParam(':password'     , $this->password);
            $sth->bindParam(':name'         , $this->name);     
            $sth->execute();
        }
       catch (PDOException  $e)
        {
            print $e->getMessage();
        }
    }   
} //end class

Thanks in advance!

[pritaeas]

public function getRecord($var)

Why not use:

public function getRecord($id)

Then you see at first glance what needs to be passed.

$sth->execute();

This function can return false indicating failure.

What I miss are function return values. True on success, perhaps an array with the error message on failure. The caller should decide whether something gets output to the screen, not the function itself.

As you get more classes beside Contacts, you can choose to build a single function for each get, insert and save, which builds the queries from a configuration array.

[extjac]

Thanks pritaeas.
Would it be possible for you to give an example of of a function from a conf array?

[pritaeas]

Okay. Here is an (partial) example. I've used a contacts table with an id and some columns. This is my database class:

class MyDB {
    protected $configuration;
    protected $pdo;

    public function __construct($tableConfig) {
        $this->configuration = $tableConfig;
        $this->pdo = new PDO('mysql:host=localhost;dbname=test', 'root', '');
    }

    public function getRecord($table, $conditions = array ()) {
        return $this->getRecords($table, $conditions, 1);
    }

    public function getRecords($table, $conditions = array (), $limit = 0) {
        $result = array ();

        $fields = implode(',', array_keys($this->configuration[$table]));
        $query = "SELECT $fields FROM $table ";

        $where = array ();
        foreach ($conditions as $column => $value) {
            $where[] = "$column = :$column";
        }
        if (count($where) > 0) {
            $where = implode(' AND ', $where);
            $query .= "WHERE $where ";
        }

        if ($limit > 0) {
            $query .= "LIMIT $limit";
        }

        $statement = $this->pdo->prepare($query);
        foreach ($conditions as $column => $value) {
            $statement->bindValue(":$column", $value, $this->configuration[$table][$column]);
        }
        if ($statement->execute()) {
            if ($row = $statement->fetchAll()) {
                $result = $row;
            }
        }

        return $result;
    }
}

And this is how to use it:

$tableConfig = array (
    'contacts' => array (
        'id' => PDO::PARAM_INT,
        'username' => PDO::PARAM_STR,
        'password' => PDO::PARAM_STR,
        'first_name' => PDO::PARAM_STR,
        'last_name' => PDO::PARAM_STR,
        'email' => PDO::PARAM_STR
    )
);

include 'MyDB.class.php';
$myDB = new MyDB($tableConfig);

print_r($myDB->getRecord('contacts', array ('id' => 1)));

I have omitted comments, as I hope the code will speak for itself. If not, please reply with your questions. For simplicity's sake, no error checking or other checks you should do when using this code. Basically, this can be extended to (re)create your tables from the configuration array, if you put more information into it. If you are interested in a more detailed example, I suggest installing SugarCRM and look how they did it. Their code also includes join's, foreign keys, etc. Perhaps I'll make a code snippet that includes the insert and update.