Seperating controller and model using transactions

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

matthijs
DevNet Master
Posts: 3360
Joined: Thu Oct 06, 2005 3:57 pm

Seperating controller and model using transactions

Post by matthijs »

Say you have a controller action for a CMS system which is used to save a blog post you just wrote. The data you're saving will not go into just one table, but several. Posts, authors, categories, etc. Now, to keep your data save in case of some unfortunate happening during the process of saving all that data to the different tables, you'd like to use transactions.

But the question is: where?
In pseudo code:

Code: Select all

class AuthorModel {
	function save(){
		$db->execute('insert into autors (name) values ($value)');
	}	
}
class PostModel{
	function save(){
		$db->execute('insert into posts (name, date) values ("$value", "$value")');
	}
}
class CatModel{
	function save(){
		$db->execute('insert into cats (name) values ("$value")');
	}
}
class BlogpostController{
	function saveaction(){
		$newauthor = new AuthorModel;
		$newauthor->name = 'henrik';
		$newpost = new PostModel;
		$newpost->setdata('some new post', '12/23/2007');
		$newcat = new catModel;
		$newcat->name = 'php';
		// now save all that using transactions
		// start transaction here???
		// commit here???
	}
}
The point is that it feels wrong that the controller would handle all this db-related stuff like starting and committing transactions. Maybe I should write a seperate Blogpost model which does all the saving for Author, Post and Cat? And in that way let the BlogpostController do something like:

Code: Select all

function save(){
    $newblog = new blog;
    $newblog->save($author, $posttitle, $postdate, $cat);
}
But then, what would the Blogpost model class look like? How would that class deal with using transactions?
Something like this?:

Code: Select all

class BlogPostModel{
function save(){
    $transaction = new pdotransaction();
    $transaction->execute('insert into autors (name) values ("henrik")');
    $transaction->execute('insert into posts (name, date) values ("some new post", "12/23/2007")');
    $transaction->execute('insert into cats (name) values ("php")');
    $transaction->commit();
}
}
But now the BlogPostModel class is something of a "god" class, doing everything for the other classes Post, Author and Cat... that doesn't seem good either.

Mind you, I'm just starting with this stuff, so I'm properly missing something obvious. But a few hints or even search terms I should look into so I can research some pattern would be helpful. I've read a lot about design patterns and understand most of them. But now that it comes to actually building something I'm at a loss somewhat. The thing is also, in each and every example I see online or in books, they deal with simple situations like saving and retrieving data from a single table. However, even if you're dealing with something relatively simple as a blog post, you're dealing with at least 3, 4 tables.
User avatar
John Cartwright
Site Admin
Posts: 11470
Joined: Tue Dec 23, 2003 2:10 am
Location: Toronto
Contact:

Post by John Cartwright »

I would keep the transaction in it's own model object like you have proposed. This way, you can keep the relationship of the controllers and models 1-1 instead 1-many.

Code: Select all

class BlogPostModel{
   function save() {
      $author = new AuthorModel('firstname', 'lastname', 'username');
      $post = new PostModel('title', 'content');
      $cat = new CatModel('4');  

      $transaction = new pdotransaction();
      $transaction->attach($author);
      $transaction->attach($post);
      $transaction->attach($cat);

      $transaction->commit();
    }
}
matthijs
DevNet Master
Posts: 3360
Joined: Thu Oct 06, 2005 3:57 pm

Post by matthijs »

Thanks for your reply and example.

Is there a reason you set the values in the constructors? Wouldn't it be more practical to use dedicated setters for the values?

Code: Select all

$author = new AuthorModel(); 
$author->seFirstname('firstname');
$author->setLastname('lastname');
etc
User avatar
John Cartwright
Site Admin
Posts: 11470
Joined: Tue Dec 23, 2003 2:10 am
Location: Toronto
Contact:

Post by John Cartwright »

When I consider data to be critical to the initialization of the object, i.e. the object requires certain information, then I would put it in the constructor (there are exceptions). As in, if the data is meant to persist throughout the lifespan of the object then I pass it in the constructor.

I don't see any harm in having the setters though, if anything it improves the interface. However, I would probably have your models extend a base class supporting __set so aren't actually required all those setters. Although, there may be special instances where you do want to support an actual setters where the data may need to be processed.
matthijs
DevNet Master
Posts: 3360
Joined: Thu Oct 06, 2005 3:57 pm

Post by matthijs »

Ok, that sounds good. I'll take that into consideration. Thanks!
matthijs
DevNet Master
Posts: 3360
Joined: Thu Oct 06, 2005 3:57 pm

Post by matthijs »

However, I would probably have your models extend a base class supporting __set so aren't actually required all those setters.
I've tried to understand how you'd do this, but there's something I dont understand:

Code: Select all

class basemodel {
	function __set($property, $value){
		$this->property = $value;
	}
	function __get($property){
		return $property;
	}
}

class widget extends basemodel {
	public $catName;
	public $catDesc;
	public function insert() { 
		// insert a new record 
		echo $this->catName . ' and ' . $this->catDesc . ' have been inserted.<br>';
	}
}
$b = new widget;
$b->catName = 'foo';
$b->catDesc = 'bar';
$b->insert();
But, as it is now, I could as well leave out the basemodel class and set the properties directly. If I change catName and catDesc to protected or private, insert() doesn't work anymore. What am I doing wrong?
User avatar
Maugrim_The_Reaper
DevNet Master
Posts: 2704
Joined: Tue Nov 02, 2004 5:43 am
Location: Ireland

Post by Maugrim_The_Reaper »

Code: Select all

class basemodel {
   function __set($property, $value){
      $this->property = $value;
   }
   function __get($property){
      return $property;
   }
}
Don't you mean:

Code: Select all

class BaseModel
{

    protected $_properties = array();

    public function __set($property, $value) 
    {
        $this->_properties[$properties] = $value;
    }

    public function __get($property) 
    {
        if (isset($this->_properties[$property])) {
            return $this->_properties[$property];
        }
        return null;
    }

    public function __isset($property) 
    {
        return isset($this->_properties[$property]);
    }

    public function __unset($property) 
    {
        unset($this->_properties[$property]);
    }

}
All the properties are in the object scope - use $this->, put all non-existing props into an array since it can help keep them segregated in some easy to extract means.

Finally - only public properties are accessible outside the object scope. Moving them to private means only the local class scope can access them, protected allows subclasses to also access them. This is why "data hiding" is often talked about:

Code: Select all

class User
{

    private $_table = 'user';
    protected $_id = 1;
    public $name = 'Paddy';

}

$user = new User;
var_dump($user->_table); // NULL
var_dump($user->_id); // NULL
var_dump($user->name); // Paddy (the only public accessible property)
matthijs
DevNet Master
Posts: 3360
Joined: Thu Oct 06, 2005 3:57 pm

Post by matthijs »

Thanks for the clarification. And for the code example.

(there's one typo though: )

Code: Select all

$this->_properties[$properties] = $value; 
// should be
$this->_properties[$property] = $value;
I understand the public/private/protected consequences. I do fail to see the advantage of using setters and getters in this situation however. Using them it means I cannot declare the variables in extended classes. Doesn't that make them harder to understand?

Code: Select all

class widget extends basemodel {
    // protected $catName; I cannot set this here
	public function insert() { 
		// insert a new record 
		echo $this->catName . ' and ' . $this->catDesc . ' have been inserted.<br>';
     // see how these variables come out of nowhere in this method
	}
}
An advantage I do see is that if you have 10 extended modelclasses, each with a few variables, using setters/getters saves a lot of typing for all get and set functions (wherever needed).
User avatar
Maugrim_The_Reaper
DevNet Master
Posts: 2704
Joined: Tue Nov 02, 2004 5:43 am
Location: Ireland

Post by Maugrim_The_Reaper »

Unit tests kill typos. Not my poor overworked brain...;)

Sure you can declare them later - the BaseModel assumes you want any number of public properties , and __set/__get encapsulates the access so you can apply getter/setter logic centrally. If you extend BaseModel:

Code: Select all

class Widget extends BaseModel {

    public $catName = 'defaultValue';

}

$widget = new Widget;
echo $widget->catName; // defaultValue
Since $catName is a defined public variable - it's excepted from __set/__get which will never get called since the class already such a named variable. So __get/__set are only called for non-existing public property requests. If you defined catName as protected, or even private, it should be accessible from within the class methods. Not sure where this is a problem in your example - if you have a full sample code to post I could see all the pieces fitting together.
matthijs
DevNet Master
Posts: 3360
Joined: Thu Oct 06, 2005 3:57 pm

Post by matthijs »

The example

Code: Select all

class BaseModel
{

    protected $_properties = array();

    public function __set($property, $value)
    {
        $this->_properties[$property] = $value;
    }

    public function __get($property)
    {
        if (isset($this->_properties[$property])) {
            return $this->_properties[$property];
        }
        return null;
    }

    public function __isset($property)
    {
        return isset($this->_properties[$property]);
    }

    public function __unset($property)
    {
        unset($this->_properties[$property]);
    }

}
class widget extends basemodel {
	protected $catName;
	protected $catDesc;
	public function insert() { 
		// insert a new record 
		echo $this->catName . ' and ' . $this->catDesc . ' have been inserted.<br>';
	}
}
$b = new widget;
$b->catName = 'foo';
$b->catDesc = 'bar';
$b->insert();  //    and    have been inserted
Now the insert() function doesn't print anything because the catName and catdesc have been declared protected.

In the meantime I found out that:

Code: Select all

..
	public function printthis($foo,$bar){
		$this->catName = $foo;
		$this->catDesc = $bar;
		echo $this->catName . ' and ' . $this->catDesc . ' have been inserted.<br>';
	}
..
$b->printthis('fooa','bara'); // fooa and bara have been inserted.
does work. So I think I understand the way it works now. However, when and why to use __set/__get is not entirely clear. Is it just to save me from typing too much? I didn't get any wiser from the few threads I read about them.
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post by feyd »

User avatar
Maugrim_The_Reaper
DevNet Master
Posts: 2704
Joined: Tue Nov 02, 2004 5:43 am
Location: Ireland

Post by Maugrim_The_Reaper »

Okay - I see now.

The problem is that __get/__set only work for public variables. Public meaning you can access them as part of an Object's API. Protected and private properties are only internally accessible (not a part of the Object's API). I always find it's the best approach to ignore the code, and start with the API you want:

Code: Select all

$b = new widget;
$b->catName = 'foo';
$b->catDesc = 'bar';
$b->insert();
Looking at the use case:

1. There's a class called Widget
2. It has two public properties, catName and catDesc
3. It implements a public insert() method

Now read no. 2 - those two properties must be public for the API to be feasible, so declaring them protected will break the desired behaviour of the object since then they won't even exist outside the object. The simplest solution is to just not declare them protected - is there any reason they must be? What was your thinking about making them protected?

The next question - about the __set/__get is how to implement it from the perspective that you only allow defined public properties (catName, catDesc) and prevent any others. The current BaseModel allows any number of public properties (not the best solution for Models since then there's no clear idea of which properties are database fields, and which aren't). You can make several changes to the code to enforce this:

Code: Select all

class BaseModel
{

    protected $_properties = array();

    /**
     * Legally allowed fields defined by a subclass.
     * Otherwise defaults to allowing no property setting
     * of anonymous properties.
     */
    protected $_fields = array();

    protected $dbh = null;

    public function __construct() 
    {
        //setup database connection and driver or PDO
    }

    public function __set($property, $value)
    {
        if (!in_array($property, $this->_fields)) {
            return false;
        }
        $this->_properties[$property] = $value;
    }

    public function __get($property)
    {
        if (!in_array($property, $this->_fields)) {
            return null;
        }
        if (isset($this->_properties[$property])) {
            return $this->_properties[$property];
        }
        return null;
    }

    public function __isset($property)
    {
        return isset($this->_properties[$property]);
    }

    public function __unset($property)
    {
        unset($this->_properties[$property]);
    }

}

class Widget
{

    public function __construct() 
    {
        parent::__construct(); // keep the parent's constructor happy
        $this->_fields[] = 'catName';
        $this->_fields[] = 'catDesc';
    }

    public function insert() 
    {
        $data = $this->_properties; //array of key=>value pairs
        
        // prepare insert statement and execute

        echo $this->catName . ' and ' . $this->catDesc . ' have been inserted.<br>';
    }

}
matthijs
DevNet Master
Posts: 3360
Joined: Thu Oct 06, 2005 3:57 pm

Post by matthijs »

Feyd and Maugrim, thanks for the links (hadn't found those) and excellent explanation. Will study them before I ask any more silly questions :)

[edit]
Ok, new questions. I see now how your example functions Maugrim.

Code: Select all

$w = new Widget;
$w->catName = 'somecategory';
$w->catDesc = 'this is the description';
$w->insert();
Does what you'd expect.

Code: Select all

$w = new Widget;
$w->catName = 'somecategory';
$w->catDesc = 'this is the description';
$w->stuff = 'drawer';// $this->stuff is not being set
$w->insert();
$this->stuff is not set, as it's not one of the fields that has been set in the constructor in the widget class.

But then rises the question: what's the whole point of extending the Basemodel. I don't understand the why. When is it useful?

In your example: how would I use the __set and __get functions then? And why?

It's just that I don't see the function of the base class. You might as well leave it out and the second (widget) class functions exactly the same. I read like 6 articles in the past hour, and they all explain sort of how it works (they repeat the manual), but never the why.
User avatar
Maugrim_The_Reaper
DevNet Master
Posts: 2704
Joined: Tue Nov 02, 2004 5:43 am
Location: Ireland

Post by Maugrim_The_Reaper »

Yes you can leave it out - but follow things through first. Where are you going to define the database connection? Where is the database connection created?

For example, I could do:

Code: Select all

class Registry {

    // ... Implements Singleton getInstance()

}

$dbh = new PDO(...);
$registry = Registry::getInstance();
$registry->dbh = $dbh;

// ...

class BaseModel
{


    protected $_dbh = null;

    public function __construct()
    {
        // Using a registry here, but a factory reference might be cleaner
        $registry = Registry::getInstance();
        $this->_dbh = $registry->dbh;
    }

    // ...

}

class Widget extends BaseModel
{

    // ...

    public insert()
    {
        $this->_dbh->prepare(...); // insert statement with ? placeholders for data
        $this->_dbh->execute();
    }

}
The point being to let the single common parent handle database instantiation via some means - a Registry here is probably not the cleanest solution since it couples BaseModel to Registry (see scottayy's earlier thread on dependency inject for config data). This leaves subclasses free to handle their own specific role as a data container (mainly). A typical refactoring after these classes are complete would be to make the insert() method generalised which is where having the property array fits in - use PDO prepare/execute pairs and it's easy work.

What this is all working up to is a general class family geared up to a simple Data Objects type design. Subclasses manage data collation, with a parent handling the concrete inserts, updates, deletes and reads (CRUD).
matthijs
DevNet Master
Posts: 3360
Joined: Thu Oct 06, 2005 3:57 pm

Post by matthijs »

Aha, yes now some light bulb starts glimmering somewhere in the tunnel :)

Thanks for your reply, again. Makes sense to try to decouple the general stuff like db access away from the model classes.

As I have it now, I have the pdo class as a singleton (very simple, just from the examples in Sweaties book). But the principle is the same. Even with a few model classes with each a few methods I can see how cluttered the code gets (and tightly coupled) with having db queries everywhere. But it is/was a good start to keep things simple for me.

I'll reread the thread from scottayy. I'm already using (parts of) Arborints' skeleton code, which has some kind of injection as well.

[edit]
Maugrim_The_Reaper wrote: What this is all working up to is a general class family geared up to a simple Data Objects type design. Subclasses manage data collation, with a parent handling the concrete inserts, updates, deletes and reads (CRUD).
Wait, I was just looking at some code from you, is this what you do in Quantum_Db_Row? In which you do something like:

Code: Select all

class Category extends Quantum_Db_Row {

	protected $_data = array(
		'name' => null,
        'description' => null
	);

    protected $_primaryKeys = array('name');

	public function __construct($fields = null, $dao = null) {
        if(!$dao)
        {
            $dao = Quantum_Db_Access::getInstance();
        }
		parent::__construct($fields, $dao, $this);
	}

}
In which the Quantum_Db_Row is an active record?
Post Reply