views:

135

answers:

6

In MVC-Model View Controller design you implement the model to work seperately and contain buisness logic, pulling information down from the database.

I struggle so much with the design of implementing a good model. I know what information needs to be pulled down from the database, I just don't know the best way to implement it. I think of the model as the program API and I overload myself with questions like

  1. What if I need to sort the fields?
  2. What if I need to select by a certain username/id?
  3. What if I need to group by a particular field?
  4. How badly will the performance be affected if I'm selecting * just incase the calling function might need any of the information pulled down?

My API/Model becomes extremely bloated, having seperate functions and queries (just slighlty tuned/changed) for each function

For example

$cart->getShoppingCart()
$cart->getShoppingCartSortByTitle()
$cart->getShoppingCartGroupByItemType()

I feel this makes the model extremely bloated and very tied, creating a lot of duplicated code. It might be better having this kind of model;

A better idea

$cart->getItems('title, price')->order_by('title');

Where 'title, price' is the mySQL fields you can select, obviously being validated by the getItems() function. That way it's not only restricted to returning certain fields.

  1. How could I achieve this?
  2. Is this truly a good model?
  3. Are there any other things you guys could suggest?
A: 

The Code Igniter framework database management allows you to perform this kind of request on the database, limiting the copy/pasted code.
See the Active Records module in their documentation

And I think that their model is quite good.

Julien N
A: 

The 'better idea' is definitely a better idea. You might consider looking at how Django implements this, because that is the approach used there. Django is written in Python which does make some things a bit easier but you should be able to use the concepts there in PHP as well (just with a bit less neatness). In summary though, making a query creates a query object that has methods like order_by. Applying these methods will change the state of the query and it is only when the query is actually performed that it needs to generate the SQL and execute it on the database.

If you are sticking to the former, you might consider using 'magic methods' with dynamic names like many existing frameworks do. For example,

getShoppingCart_groupby
getShoppingCart_orderby

You would have a single 'catch-all' method with a dynamic argument list that reads the name of the function that was called and performs the behaviour required if it is valid (and throws the standard 'method not found' error if it is not). This is basically the same as what you are doing now, but it would tidy up the code a lot and unclutter your model. You'll need PHP5 for this and you're looking for the __call magic method.

Julien has mentioned Code igniter in his answer - writing good models is very hard so typically you will be best using an existing framework. (but it is fun to try!)

adamnfish
A: 

First off, make sure that all of those "questions" you ask yourself concern features that your application actually needs right now. One of the biggest issues I've seen with designing new projects is speculative design. Only add what you need. Write unit tests for what you add. When you get to the point where you need the extra features, refactor to a better design if necessary.

If you do need all of the features up front, I still recommend the refactoring approach. Implement a few of the features that you can see being similar or contributing to the "bloat". When you're done, take a step back and see if you can refactor to something that is more elegant or something that distributes the responsibility more evenly between different objects and/or methods. Then continue. Various "pattern" and "refactoring" books will help you a lot here.

rr
+1  A: 

Use object-relational mapping (ORM)...

Try the Doctrine ORM project.

Another solution is CodeIgniter, it has the best active records library out there. Very helpful.

If you still decide to write own class then use PHP5's method chaining. Syntax would be prettier...

Otar
A: 

First and foremost you should take into consideration is:

  • There is no good general model. Every projects needs its own model.
  • easy to read, managable code
  • do not repeat the same code (or queries), so if you have a function for a certain task and you want it to be ordered another way, modify the function itself and do not clone it
  • Use complex data structures, like arrays or objects to send data to a function, so you don't have to always modify the parameters a function needs
  • resource usage. The more you want it to be all around, general solution the more resource it will use.

How badly will the performance be affected if I'm selecting * just incase the calling function might need any of the information pulled down?

It depends on the load of your website. Most of the time (if you don't pull big blobs and text) * is ok, but when resources are scarce, you have to specify the columns. So you can save some IO time.


I feel this makes the model extremely bloated and very tied, creating a lot of duplicated code. It might be better having this kind of model;

Maybe try this:

First of all, for complex queries, I use this class I made long time ago for MySQL. It helps a helluva lot.

class sqlAssembler
{
    private $data = array();
    var $S = array();
    var $F = array();
    var $W = array();
    var $G = array();
    var $H = array();
    var $O = array();
    var $L = array();

    //Clause abbreviations
    var $clauselist = array
    (
    'S' => 'SELECT',
    'F' => 'FROM',
    'W' => 'WHERE',
    'G' => 'GROUP BY',
    'H' => 'HAVING',
    'O' => 'ORDER BY',
    'L' => 'LIMIT'
    );

    //Default clause separators
    var $clausesep = array
    (
    'S' => ',',
    'F' => ',',
    'W' => ' AND ',
    'G' => ',',
    'H' => ' AND ',
    'O' => ',',
    'L' => ''
    );

    function gen()
    {
        $tmp = '';

        foreach ( $this->clauselist as $area => $clause )
        {
            if ( count($this->{$area}) )
            {
                $tmp .= ($clause != 'S' ? ' ' : '') . $clause . ' ';
                for ($i=0; $i < count($this->{$area}); $i++) 
                {
                    //echo $area = (string)$area;
                    $tmp .= $this->{$area}[$i];
                } //for
            } //if
        } //foreach

        return $tmp;
    } //function


    function genSection($area, $showsection = 0)
    {
        $tmp = '';
        if ( count($this->{$area}) )
        {
            for ($i=0; $i < count($this->{$area}); $i++) 
            {
                $tmp .= $this->{$area}[$i];
            } //for
        } //if

        return $tmp;
    } //function

    function clear()
    {
        foreach ($this as $area => $v)
        {
            //We only care about uppercase variables... do not declare any else variable with ALL UPPERCASE since it will be purged
            if (ctype_upper($area))
            {
                if ($area == 'L')
                    $this->$area = '';
                else
                    $this->$area = array();
            } //if
        } //foreach
    } //function

    public function add($area, $str, $criteria = 1, $sep = '#')
    {
        if ($criteria)
        {
            if ($sep == '#')
                $sep = $this->clausesep[$area];

            //Postgres' OFFSET should be set like: $str = '25 OFFSET 0'
            //Not very neat I know, but fuck it
            if ($area == 'L')
            {
                $this->{$area} = array();   
            } //if

            //$ref = $this->$area;
            $this->{$area}[] = (count($this->$area) ? $sep : '').$str;

            return count($this->$area)-1;
        } //if
    } //function

    public function del($area,$index)
    {
        if ( isset($this->{$area}[$index]) )
            unset($this->{$area}[$index]);
        else
            trigger_error("Index nr. {$index} not found in {$area}!",E_USER_ERROR);
    } //function

//-*-* MAGIC CHAIN FUNCTIONS 

    public function S($str,$criteria = 1,$sep = '#')
    {
        $this->add(__FUNCTION__,$str,$criteria,$sep);
        return $this;
    } //function

    public function F($str,$criteria = 1,$sep = '#')
    {
        $this->add(__FUNCTION__,$str,$criteria,$sep);
        return $this;
    } //function

    public function W($str,$criteria = 1,$sep = '#')
    {
        $this->add(__FUNCTION__,$str,$criteria,$sep);
        return $this;
    } //function

    public function G($str,$criteria = 1,$sep = '#')
    {
        $this->add(__FUNCTION__,$str,$criteria,$sep);
        return $this;
    } //function

    public function H($str,$criteria = 1,$sep = '#')
    {
        $this->add(__FUNCTION__,$str,$criteria,$sep);
        return $this;
    } //function

    public function O($str,$criteria = 1,$sep = '#')
    {
        $this->add(__FUNCTION__,$str,$criteria,$sep);
        return $this;
    } //function

    public function L($str,$criteria = 1,$sep = '#')
    {
        $this->add(__FUNCTION__,$str,$criteria,$sep);
        return $this;
    } //function
} //_sql

Maybe try this:

function getShoppingCart($d)
{
    $xx = new sqlAssembler();

    $xx->S('*')->
    F('items')->
    //Notice, that we specified a criteria... if $d['id_item'] exists it will be joined to the WHERE clause, if not it will be left out
    W("(id_item > '{$d[id_item]}')",$d['id_item'])->
    //Same here
    O("dt DESC",$d['date'])
    $sql = echo $xx->gen();

    //id_item = 11, date = 2009-11-12
    //$sql = "SELECT * FROM items WHERE (id_item > '11') ORDER BY dt DESC";

    //id_item = null, date = null
    //$sql = "SELECT * FROM items";

    $data = sqlArray($sql);

    //... handle data
}
Jauzsika