Thinking about MVC models

Not for 'how-to' coding questions but PHP theory instead, this forum is here for those of us who wish to learn about design aspects of programming with PHP.

Moderator: General Moderators

Post Reply
Gambler
Forum Contributor
Posts: 246
Joined: Thu Dec 08, 2005 7:10 pm

Thinking about MVC models

Post by Gambler »

I took my stab at article model.

Code: Select all

<?php
class Article(){
    static protected $_statuses = array('', 'deleted', 'archived', 'updatedByEditor', 'updatedByAuthor', 'published', 'hidden');
    
    public __construct($attrs = array(), $save = FALSE){
        foreach ($attrs as $name => $value) {
            $this->attrs[$name] = $value;
        }
        if ($save) {
            $this->save();
        }
    }
    
    function save(){
        if (isset($this->id)) {
            $this->update();
        } else {
            $this->create();
        }
    }
    
    function update(){
        $attrs = array();
        foreach ($this as $name => $var) {
            if ($name{0} != '_') { //underscore denotes "system" attributes
                $attrs[$name] = $var;
            }
        }
        unset($attrs['id']);
        
        self::checkId($this->id);
        if (isset($attrs['category'])) {
            self::checkCategory($attrs['category']);
        }
        if (isset($attrs['status'])) {
            self::checkStatus($attrs['status']);
        }
        if (isset($attrs['title'])) {
            self::checkTitle($attrs['title']);
        }
        if (isset($attrs['preface'])) {
            self::checkPreface($attrs['preface']);
        }
        if (isset($attrs['text'])) {
            self::checkText($attrs['text']);
        }
        if (isset($attrs['time'])) {
            $attrs['time'] = time2iso($attrs['time']);
        }
        if (isset($attrs['info'])) {
            $attrs['info'] = Ori::fold($attrs['info']);
        }
        
        $set = QMaster::makeUpdate($attrs);
        $query = "UPDATE articles SET $set
            WHERE `id` = '$id'";
        $result = mysql_query($query);
        if (!$result) {
            throw new Exception("Invalid query: [<i>$query</i>] ".mysql_error());
        }
        if (mysql_affected_rows() < 0) {
            return FALSE;
        }
        return TRUE;
    }
    
    function create(){
        $attrs = array();
        foreach ($this as $name => $var) {
            if ($name{0} != '_') {
                $attrs[$name] = $var;
            }
        }
        
        self::checkCategory($attrs['category']);
        self::checkStatus($attrs['status']);
        self::checkTitle($attrs['title']);
        self::checkPreface($attrs['preface']);
        self::checkText($attrs['text']);
        
        $attrs['time'] = time2iso($attrs['time']);
        $attrs['info'] = Ori::fold($attrs['info']);
        list($names, $values) = QMaster::makeInsert($attrs);
        
        $query = "INSERT INTO `articles` ($names) VALUES ($values)";
        $result = mysql_query($query);
        if (!$result) {
            throw new Exception("Invalid query: [<i>$query</i>] ".mysql_error());
        }
        if ($this->attrs['id'] == 0) {
            return FALSE;
        }
        $this->id = mysql_insert_id();
        return $this->id;        
    }
    
    static function checkId($value){
        if ((int) $value < 1) {
            throw new UserException('Invalid id');
        }
    }
    
    static function checkCategory($value){
        if ((int) $value < 0) {
            throw new UserException('Invalid category');
        }
    }
    
    static function checkStatus($value){
        if (!in_array($value, self::$_statuses)) {
            throw new UserException('Invalid status');
        }
    }
    
    static function checkTitle($value){
        if (strlen($value) > 100 || empty($value) 
        || strpos($value, "\n") !== FALSE) {
            throw new UserException('Invalid title');
        }
    }
    
    static function checkPreface($value){
        if (strlen($value) > 3000) {
            throw new UserException('Invalid preface');
        }
    }
    
    static function checkText($value){
        if (strlen($value) > 200000) {
            throw new UserException('Invalid text');
        }
    }
    
    static function get($id){
        self::checkId($id);
        
        $query = "SELECT * FROM `articles` WHERE `id` = '$id'";
        $result = mysql_query($query);
        if (!$result) {
            throw new Exception("Invalid query: [<i>$query</i>] ".mysql_error());
        }
        $article = mysql_fetch_assoc($result);
        if (!$article) {
            return NULL;
        }
        $article['time'] = iso2unix();
        $article['info'] = Ori::unfold($article['info']);
        return new Article($article);
    }
    
    static function getPage($pageSize, $pageNum = 0, $where = ''){
        $pageSize = (int) $pageSize;
        if ($pageSize < 1) {
            throw new Exception("Invalid page size");
        }
        $offset = $pageNum * $pageSize;
        
        $query = "SELECT * FROM `articles`";
        if (!empty($where)) {
            $query .= " WHERE $where";
        }
        $query .= " LIMIT $pageSize";
        if ($offset > 0) {
            $query .= " OFFSET $offset";
        }
        
        $result = mysql_query($query);
        if (!$result) {
            throw new Exception("Invalid query: [<i>$query</i>] ".mysql_error());
        }
        $articles = array();
        while ($article = mysql_fetch_assoc($result)) {
            $article['time'] = iso2unix();
            $article['info'] = Ori::unfold($article['info']);
            $articles[] = new Article($article);
        }
        return $articles;
    }
    
    static function delete($id){
        self::checkId($id);
        
        $query = "DELETE FROM `articles`
            WHERE `id` = '$id'";
        $result = mysql_query($query);
        if (!$result) {
            throw new Exception("Invalid query: [<i>$query</i>] ".mysql_error());
        }
        if (mysql_affected_rows() < 0) {
            return FALSE;
        } else {
            return TRUE;
        }
    }
}
?>
Feels clumsy. Bunch of static methods that return arrays are so much simpler. In fact, I decided to use a bunch of static methods for the time being. Two main problems with OO approach are that I need to synchronize object with database in at least 3 different ways (create, update, read) and that I want to avoid redundant typing, redundant quesries and redundant method calls. Simple actions should be coded simple.

Some people do it like this:
$o = new Article();
$article->setX($x);
$article->setY($y);
$article->create();
Lots of redundant typing and methods calls.

Other way:
$id = Article::create(array('x'=>$x, 'y'=>$y));
IMO better, but later I will need to update the same article in some way.

So I do something like this, right?
$article = Article::get($id); //query
$article->bla = 11;
$article->update(); //query
But that's 1 extra query. Not lethal, but it feels wrong.

Another possibility:
$article = new Article(); //no query
$article->bla = 11;
$article->mergeWith($id); //query
The problem is that good object should represent something.

Almost the same thing:
$correction = Article::correctionsFor($id); //no query
$correction->bla = 11;
$correction->perform(); //query
Do I need to create separate ArticleCorrection class? Smells of incorrectly done Java program.

And another one:
$correct = array('bla' => 11);
Article::update($id, $correct);
Looks okay, but that's not object-oriented.

The "right" way:
$article = new Article($id); //no query
$article->bla = 11; //no query, but the class notes that article with creatain $id is updated
//...
$article->__destruct(); //that's where we do cleanup, and save article
//or
$someOtheAttr = $article->someOtheAttr; //article updates db, then reads db to sychronize it's attributes
The only problem with this approach is that class will be bloated, and execution flow would be quite complex. This is how I would do it in Java, but we're speaking about PHP.

Any ideas how to do it "properly"? Am I overanalyzing insignificant problem?
User avatar
Ambush Commander
DevNet Master
Posts: 3698
Joined: Mon Oct 25, 2004 9:29 pm
Location: New Jersey, US

Post by Ambush Commander »

I think what you want is a Unit of Work. And stop using an Active Record.
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Post by Christopher »

Perhaps something more like:

Code: Select all

$article = $articlemapper->load($id);
$article->bla = 11;
$articlemapper->save($article);
(#10850)
Gambler
Forum Contributor
Posts: 246
Joined: Thu Dec 08, 2005 7:10 pm

Post by Gambler »

Hm, I'm not using Active Record, and Unit Of Works will not help here. Data mapper is not what I'm looking for either.

What I really need is a code for some real-life high-performance model in PHP just to see how others implement it. Something like website user model. Not a textbook example. All I've seen so far is either overly simplified, or bloated.

I've seen things like this:

Code: Select all

<?php
class UserModel{
    private $id;
    private $name;
    private $password;
    private $email;
    private $regDate;
    private $icq;
    private $aol;
    private $yim;
    private $description;
    private $activationCode;
    //...

    __construct($attrs){
        $this->id = $attrs['id'];
        $this->name = $attrs['name'];
        $this->password = md5($attrs['password']);
        //...
    }
    
    function update(){
        $id = mysql_real_escape_string($this->id);
        $name = mysql_real_escape_string($this->name);
        $password = mysql_real_escape_string($this->password);
        //...
       $sql = "UPDATE user SET name='$name', password='$password'
       --...
       ";
    }
}
?>
Does anyone feels compelled to code like this? I don't.
User avatar
Maugrim_The_Reaper
DevNet Master
Posts: 2704
Joined: Tue Nov 02, 2004 5:43 am
Location: Ireland

Post by Maugrim_The_Reaper »

Its not ActiveRecord...
What I really need is a code for some real-life high-performance model in PHP just to see how others implement it.
The system I use is laid out in the tutorial linked from my signature in all its (possibly gory) details... It's missing a few features I need to add (because I do use a slightly more complex variant of it) but they're relatively simple to do. I needed something for ORM that would perform within a certain limit, and it gets me there. I also did not want to maintain SQL statements for CRUD, nor did I want to hand type anything outside specific convenience methods.

Main difference I suppose is that I only have one class to handle all CRUD operations which include getByPk(), getRow(), getAll(), save() and delete(). I have no separate update() method - classes are aware when they are updated and this informs save() of whether to use a CREATE/UPDATE call. If not changed, erroneous save() calls are ignored. Sounding a bit like UoW functionality?

Table rows are then representing by simplistic data objects which the DAO can perform CRUD with (the DAO cares not what type the data object is). The data objects are in fact so predictable I just generate them automatically from the database.
User avatar
Ambush Commander
DevNet Master
Posts: 3698
Joined: Mon Oct 25, 2004 9:29 pm
Location: New Jersey, US

Post by Ambush Commander »

Hm, I'm not using Active Record
Well, it certainly looks like your using active record. All your SQL is wrapped up with the domain logic (arguably, it's seperated by functions, but they're all lumped in the same class). Plus, you have stuff like $object->save() which tells us that the object knows about the database.

The Unit of Work would be nice because it implicitly saves objects without needing you to manually call ->__destruct or anything like that.

A mapper is a good idea because it makes the domain objects not know about the database mapping, which, arguable, they shouldn't know about. Especially important when the in-memory object doesn't map neatly to the database, and things like foreign-keys must be created.

You probably also want an Identity Map to prevent duplication. Whenever the mapper grabs an object from the database, it registers the object to the map. On subsequent requests for the object, the in-memory object is given, saving a query. Then, the unit of work saves everything in one sweep.

All these things are real life.

PS. I'd create a DB wrapper object. It would save a whole lot of calls to mysql_real_escape_string
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Post by Christopher »

Gambler wrote:Hm, I'm not using Active Record, and Unit Of Works will not help here. Data mapper is not what I'm looking for either.

What I really need is a code for some real-life high-performance model in PHP just to see how others implement it. Something like website user model. Not a textbook example. All I've seen so far is either overly simplified, or bloated.

I've seen things like this:

Does anyone feels compelled to code like this? I don't.
That looks like a pretty standard Gateway class ... so the answer is yes -- lots of people.

I sounds like you don't like everything you have seen, so post some code of how you think it should be done.
(#10850)
Gambler
Forum Contributor
Posts: 246
Joined: Thu Dec 08, 2005 7:10 pm

Post by Gambler »

I sounds like you don't like everything you have seen, so post some code of how you think it should be done.
Here is my latest variation. But I can't say I like it either. It does what I want it to do, but without any elegance. I want to highlight the use of validate and normalize functions. They allow me to create exceptions that list all invalid fields. In theory I can tell the user which fields are invalid and why. Also, mixing up DB and business logic allows me to create things like info field, so I can add new attributes without changing database schema.

Hm... Speaking of which, aren't databses supposed to keep track of business logic?

Code: Select all

<?php
class User{
    const UNKNOWN = 1;
    const SYSTEM = 2;
    
    const SALT = 's@lt';
    
    protected $attrs = array(
        'id' => NULL,
        'name' => NULL,
        'pass' => NULL,
        'email' => NULL,
        'regDate' => NULL,
    );
    protected $info;

    function __set($name, $value){
        if (key_exists($name, $this->attrs)) {
            $this->attrs[$name] = $value;
        } else {
            $this->info[$name] = $value;
        }
    }
    
    function __get($name){
        if (key_exists($name, $this->attrs)) {
            return $this->attrs[$name];
        } else {
            return $this->info[$name];
        }
    }
    
    function __unset($name){
        if (key_exists($name, $this->attrs)) {
            $this->attrs[$name] = NULL;
        } else {
            unset($this->info[$name]);
        }
    }
    
    function __isset($name){
        if (key_exists($name, $this->attrs)) {
            return isset($this->attrs[$name]);
        } else {
            return isset($this->info[$name]);
        }
    }
    
    function validate(){
        $attrs = &$this->attrs;
        $errors = array();
        if (isset($attrs['id']) && (int) $attrs['id'] < 1) {
            $errors[] = "Invalid id";
        }
        if (!preg_match('/^[\w\d\'\-\. ]{1,50}$/', $attrs['name'])) {
            $errors[] = "Invalid name";
        }
        if (isset($attrs['password']) && strlen($attrs['password']) < 6) {
            $errors[] = "Invalid password";
        }
        if (!preg_match('/^[.0-9a-z_+-]+@([0-9a-z-]+\.)+[0-9a-z]+$/i', $attrs['email'])) {
            $errors[] = "Invalid email";
        }
        if (!empty($errors)) {
            throw new Exception(join("\n", $errors));
        }
    }
    
    function normalize(){
        if (isset($this->info['password'])) {
            if (empty($this->info['activationCode'])) {
                $this->attrs['pass'] = sha1($this->info['password'].self::SALT);
            } else {
                $this->info['tempPass'] = sha1($this->info['password'].self::SALT);
            }
        }
        unset($this->info['password']);
    }
    
    function deactivate(){
        if (isset($this->info['activationCode'])) {
            return FALSE;
        }
        $this->info['activationCode'] = sha1(uniqid(rand(), TRUE));
        $this->info['tempPass'] = $this->attrs['pass'];
        $this->attrs['pass'] = NULL;
        return TRUE;
    }
    
    function activate($code = NULL){
        if (@$this->info['activationCode'] != $code) {
            return FALSE;
        }
        $this->attrs['pass'] = $this->info['tempPass'];
        unset($this->info['tempPass'], $this->info['activationCode']);
        return TRUE;
    }
    
    function create(){
        $this->validate();
        $this->normalize();
        $attrs = array_map('mysql_real_escape_string', $this->attrs);
        $info = mysql_real_escape_string(Ori::fold($this->info, Ori::NO_EMPTY));
        
        $query = "INSERT INTO `users` (`name`, `pass`, `email`, `regTime`, `info`)
            VALUES ('$attrs[name]', '$attrs[pass]', '$attrs[email]', NOW(), '$info')";
        $result = mysql_query($query);
        if (!$result) {
            throw new Exception("Invalid query: [<i>$query</i>] ".mysql_error());
        }
        $this->id = mysql_insert_id();
    }
    
    function update(){
        $this->validate();
        $this->normalize();
        $attrs = array_map('mysql_real_escape_string', $this->attrs);
        $info = mysql_real_escape_string(Ori::fold($this->info, Ori::NO_EMPTY));
        
        $query = "UPDATE `users` SET 
            `name` = '$attrs[name]', 
            `pass` = '$attrs[pass]',
            `email` = '$attrs[email]',
            `info` = '$info'
            WHERE `id` = '$attrs[id]'";
        $result = mysql_query($query);
        if (!$result) {
            throw new Exception("Invalid query: [<i>$query</i>] ".mysql_error());
        }
        if (mysql_affected_rows() > 0) {
            return TRUE;
        } else {
            return FALSE;
        }
    }
    
    static function delete($id){
        $id = mysql_real_escape_string($id);
        $query = "DELETE FROM `users`
            WHERE `id` = '$id'";
        $result = mysql_query($query);
        if (!$result) {
            throw new Exception("Invalid query: [<i>$query</i>] ".mysql_error());
        }
        if (mysql_affected_rows() > 0) {
            return TRUE;
        } else {
            return FALSE;
        }
    }
    
    static function selectById($id){
        $id = mysql_escape_string($id);
        $query = "SELECT *
            FROM users
            WHERE `id` = '$id'";
        $result = mysql_query($query);
        if (!$result) {
            throw new Exception("Invalid query: [<i>$query</i>] ".mysql_error());
        }
        $attrs = mysql_fetch_assoc($result);
        if ($attrs == NULL) {
            return NULL;
        }
        $user = new self();
        $user->info = Ori::unfold($attrs['info']);
        unset($attrs['info']);
        $user->attrs = $attrs;
        return $user;
    }
    
    static function selectByName($name){
        $name = mysql_escape_string($name);
        $query = "SELECT *
            FROM users
            WHERE `name` = '$name'";
        $result = mysql_query($query);
        if (!$result) {
            throw new Exception("Invalid query: [<i>$query</i>] ".mysql_error());
        }
        $attrs = mysql_fetch_assoc($result);
        if ($attrs == NULL) {
            return NULL;
        }
        $user = new self();
        $user->info = Ori::unfold($attrs['info']);
        unset($attrs['info']);
        $user->attrs = $attrs;
        return $user;
    }
    
    static function selectPage($pageSize = 0, $pageNum = 0, $where = '', $order = ''){
        $id = mysql_escape_string($id);
        $query = "SELECT *
            FROM `users`";
        if (!empty($where)) {
            $query .= " WHERE $where";
        }
        if (!empty($order)) {
            $query .= " ORDER BY $order";
        }
        $pageSize = (int) $pageSize;
        if ($pageSize > 0) {
            $offset = $pageNum * $pageSize;
            $query .= " LIMIT $pageSize";
            if ($offset > 0) {
                $query .= " OFFSET $offset";
            }
        }
        
        $result = mysql_query($query);
        if (!$result) {
            throw new Exception("Invalid query: [<i>$query</i>] ".mysql_error());
        }
        $attrs = mysql_fetch_assoc($result);
        if ($attrs == NULL) {
            return NULL;
        }
        $user = new self();
        $user->info = Ori::unfold($attrs['info']);
        unset($attrs['info']);
        $user->attrs = $attrs;
        return $user;
    }
    
    static function register($name, $password, $email){
        $user = new self();
        $user->name = $name;
        $user->password = $password;
        $user->email = $email;
        $user->deactivate();
        $user->create();
        return $user;
    }
}
?>
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Post by Christopher »

This looks like a pretty standard Gateway style Model. The usual method names are slightly different (findBy rather than selectBy, and insert rather than create) but it looks good to me. I might add the validation part by using a separate validator class rather than built-in -- just to make that code a separate unit for testing. All in all it looks like nice clear Model code -- I could pick up that code and use it immediately.
(#10850)
Post Reply