5

Let me start with a little introduction. I am in the process of learning OOP in PHP. I have also researched Design Patterns but have not yet fully grasped the different types of concepts. I am at the stage where every few months I realise that I am not doing things the correct way and have to change my style. This is so frustrating. Therefore I would like to find out the correct way of doing things once and for all. I have tried to fully read up on Stackoverflow about the following topics:

ORM
Data Mapper
Singleton
Globals are evil
Everything related

However I am still not clear about a few things. I am posting my code here in a clear and concise way and hope that people can point out both the good practices and the bad ones. I will list all my questions at the end.

Please do not close as a duplicate, I have honestly searched through almost every question on the topic but I still want to know a few things which I have not been able to clarify. Sorry it is so long but I have tried to organize it so it should read well!

I will start by posting the essentials of my Database class.

Database.php

<?php 


class DatabaseMySQL{

    private static $dbh;

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

    public function open_connection(){
        if(!self::$dbh){
            return (self::$dbh = new PDO(DB_TYPE.':host='.DB_HOST.';dbname='.DB_NAME, DB_USER,DB_PASSWORD)) ? true : false;
        }
        return true;
    }

    public function query($sql, $params=array()){
        $this->last_query = $sql;
        $stmt = self::$dbh->prepare($sql);
        $result = $stmt->execute($params);  
        return $result ? $stmt : $stmt->errorInfo();
    }

    public function fetch_all($results, $class_name=''){
        return $results->fetchAll(PDO::FETCH_CLASS, $class_name);
    }

}

?>

This is my Database class file. This class allows me to create as many instances as I like of this class and it will reuse the already instantiated PDO object stored as a Static property of the class. It also fetches the data from the result set using PDO to get the data as objects of a specified class.

My next file I have is my class that all my other classes inherit from. I have called it MainModel. I have no idea if this follows convention or not.

MainModel.php

<?php



abstract class MainModel{

    protected static $table;

    public function __construct($array=array()){
        $this->assign_known_properties($array);
    }

    public function assign_known_properties($array){
        foreach($array as $key=>$value){
            $this->$key = $value;
        }
    }

    public static function find_by_id($id){
        $db = new DatabaseMySQL();
        self::intialise_table_name();
        $id = (int) $id;
        $sql = "SELECT * FROM ".static::$table." ";
        $sql .= "WHERE id = {$id} ";    
        $result = self::find_by_sql($sql);      
        return array_shift($result);
    }

    public static function find_all(){
        $db = new DatabaseMySQL();
        self::intialise_table_name();
        $sql = "SELECT * FROM ".self::$table." ";
        return self::find_by_sql($sql);
    }

    public static function fetch_as_objects($results){
        $db = new DatabaseMySQL();
        $called_class = get_called_class();
        $results = $db->fetch_all($results, $called_class);
        return $results;
    }

    public static function find_by_sql($sql){
        $db = new DatabaseMySQL();
        $results = $db->query($sql);
        return $results ? self::fetch_as_objects($results) : false; 
    }

    public static function intialise_table_name(){
        $called_class = get_called_class();
        static::$table = strtolower($called_class).'s';
    }

    public function get_table_fields(){
        self::intialise_table_name();
        $sql = "SHOW FIELDS FROM ".static::$table." ";
        return self::find_by_sql($sql);
    }

    public function set_table_details(){
        $fields = $this->get_table_fields();
        $total = count($fields);
        $array = array();
        foreach($fields as $object){
            $array [] = $object->Field;
        }
        $this->table_details = array('objects'=>$fields,'array'=>$array,'total'=>$total);
        $this->set_placeholders_for_new_record();
        $this->set_properties_as_array();
        $this->set_properties_as_array(true);
    }

    public function set_properties_as_array($assoc=false){
        $array = array();
        if (!$assoc){
            foreach($this->table_details['array'] as $field){
                if(isset($this->$field)){
                    $array [] = $this->$field;
                }else{
                    $array [] = NULL;
                }
            }
            $this->table_details['values'] = $array;
        }else{
            foreach($this->table_details['array'] as $field){
                if(isset($this->$field)){
                    $array[$field] = $this->$field;
                }else{
                    $array [$field] = NULL;
                }
            }
            $this->table_details['assoc_values'] = $array;
        }
    }

    public function set_placeholders_for_new_record(){
        $string = '';
        for($i=0; $i<$this->table_details['total']; $i++){
            $string .= '? ';
            if(($i+1) != $this->table_details['total'] ){
                $string .= ", ";
            }
        }
        $this->table_details['placeholders'] = $string;
    }

    public function create(){
        $db = new DatabaseMySQL();
        $this->set_table_details();
        $sql = "INSERT INTO ".static::$table." ";
        $sql .= " VALUES({$this->table_details['placeholders']}) ";
        $result = $db->query($sql, $this->table_details['values']);

        // If array is returned then there was an error.
        return is_array($result) ? $result : $db->insert_id();
    }

    public function update(){
        $db = new DatabaseMySQL();
        $this->set_table_details();
        $sql = "UPDATE ".static::$table." ";
        $sql .= " SET ";
            $count = 1;
            foreach($this->table_details['array'] as $field){
                $sql .= "{$field} = :{$field} ";
                if($count < $this->table_details['total']){
                    $sql .= ", ";
                }
                $count++;
            }

        $sql .= " WHERE id = {$this->id} ";
        $sql .= " LIMIT 1 ";
        $result = $db->query($sql, $this->table_details['assoc_values']);
        return $result;
    }

    public function save(){
        return isset($this->id) ? $this->update() : $this->create();
    }
}


?>

To summarise this file. I use static methods like find_by_id($int) that generate objects of the called class dynamically. I am using Late Static Bindings to gain access to the name of the called class and I use the $stmt->fetchAll(PDO::FETCH_CLASS, $class_name) to instantiate these objects with the data from the database automatically converted into objects.

In each static method I instantiate an instance of the DatabaseMySQL class. In each static method I set the correct static $table name to be used in the SQL queries dynmically by getting the name of the class and appending an s to it. So if my class was User, that would make the table name users.

In my constructer I have placed an optional array which can be used to insert some variables as properties of an object as it is created. This way everything is done dynamically which leaves me to the final stage of my project. My inheritance classes.

User.php

class User extends MainModel{

}

Question.php

class Question extends MainModel{

}

What I do now is simple. I can say:

$user = new User(array('username'=>'John Doe'));
echo $user->username; // prints *John Doe*
$user->save(); // saves (or updates) the user into the database.

I can retrieve a user with a static call to User.

$user = User::find_by_id(1);
echo $user->username; // prints users name

So now for my questions:

  • 1) What name would you call this design pattern (if it even is one at all).Data Mapper? Dependency Injection? Domain Model (whatever that is)? Data Access Layer?

  • 2) As it stands now is this implementation considered well structured?

  • 3) If it is good, is there a naming convention I should be aware of that is missing in my code?

  • 4) If it is considered good, would you mind pointing out what particularly you liked so I will know which part to definitely keep?

  • 5) If you dont think it is good would you care to give a detailed explanation why this is so?

  • 6) Should my create, update, delete which are all methods of my objects be in the same class as my find_by_id, find_all which are only called statically and actually return objects. If they should be in two different classes, how do I go about that?

  • 7) Why is everyone else using $load->('UserClass') functions and fancy words like mappers and I havent yet needed them once?

Zevi Sternlicht
  • 5,399
  • 19
  • 31
  • 4
    This is fits more here: http://codereview.stackexchange.com/ – Ilia Ross Aug 27 '12 at 19:21
  • 1
    @IliaRostovtsev thanks i didnt know that, but I still want an answer from the experts and I think they are mainly here! – Zevi Sternlicht Aug 27 '12 at 19:23
  • 3
    No. Don't just read up on the topics (and I discourage using SO as a _primary_ reference). Get experience, don't be afraid to get your hands dirty (this question is a good start, tho). Know that there is no black and white. I don't know the context but globals are not necessarily evil, just horrible if misused. Same thing for ORM and Singleton and, well, everything---sometimes they're good practices but not always the best. – skytreader Aug 27 '12 at 19:32
  • 1
    The fact that you realise every few months that you're doing things wrong is actually a good thing in itself, it means you're growing and learning as a programmer. That's something you always should be striving for. It also means you're gaining experience and learning from it. – GordonM Aug 27 '12 at 19:55
  • @gordonM thanks for the encouragement, but i would prefer to just know it all already. Was acutally perusin your answer just before http://stackoverflow.com/a/9227695/1624960 . Can you help me with my problems? – Zevi Sternlicht Aug 27 '12 at 19:57

1 Answers1

1

The solution you've come up with is called "Active Record"; for benefits and drawbacks, you could read Martin Fowler's book Patterns of Enterprise Architecture, as well as many of the conversations you can find on the internet.

Fwiw, my personal point of view is that this is a perfectly fine way of building database-driven applications where the primary business logic is reading and writing to the database; it tends to become a bit burdensome when building more complex business logic.

Also, Stack Overflow is not for "please discuss this topic" style questions - it's for answers to which there is an objectively true answer. So, your question 1 fits, but the others are not really suitable for SO...

Neville Kuyt
  • 29,247
  • 1
  • 37
  • 52
  • Thanks Neville for taking the time, I didnt think my question was the discuss this topic style, what about question 3 and question 6? – Zevi Sternlicht Aug 27 '12 at 21:21
  • @InGodITrust Maybe you should ask God =p. Ok I'll answer 3 and 6: For naming convention and namespacing read about [PSR-0](https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-0.md), which is the standard right now. About your methods, it's ok to have them all in the same class. You should also think about separating the DBAL from the ORM. You should separate the connection part, from the query building part, and the entities/relationships part. This is all good for learning, I've done it too. But for real projects you should use well tested libraries like Doctrine2 or Symfony2. – ChocoDeveloper Aug 30 '12 at 19:54
  • @ChocoDeveloper i did ask him and am still waiting for a reply. Thanks for the tips, great link you gave! – Zevi Sternlicht Aug 30 '12 at 20:35