Welcome Guest, Not a member yet? Register   Sign In
What to put in controller and what to put in model?
#1

[eluser]mvdg27[/eluser]
Hi,

After reading some stuff about Models on this forum, I decided to start using them as well. Before I simply put everything in my controllers.

I get the idea of Models, but am running into some day-to-day decisions about what to put in the model, and what to put in the controller.

1) I have a funtion in the Model returning one database row. After I want to make sure, there is no html and no slashes in the data. So I run:

Code:
foreach($row as $key=>$value) {
  $data[$key] = stripslashes(strip_tags($value));
}

Should I do this in the model or the controller?

2) Before inserting data into the database I check it for validity. A very simple example:

Code:
if(!$this->input->post('name')) $errors['name'] = $this->lang->line('error_name');
// some more checks

if(!count($errors)) {
  // prepare database input
  $db_input['name'] = strip_tags($this->input->post('name'));
            
  // friendly url
  $db_input['friendly_url'] = url_title(strip_tags($this->input->post('name')));

  // insert/ update statement here with $db_input as variable
  return true;
}
else {
  return false
}

This is currently all in my controller, but I'm in doubt which part should stay in the controller and which part should go to the model.

Probably there are more of these kind of decisions. And off course I understand there is no single right way. But as I'm using Models for the first time, I'd appreciate some advice here. If one of you has experience with other kinds of issues of doubt, please let them know as well (and how you solved them). I'm sure either myself or someone else will run into the same issue as well, at one point Smile

Thanks! -Michiel
#2

[eluser]jalalski[/eluser]
Hi Michiel

It's a big question, what goes in a Model, what goes in a Controller. Or whether to use Models and Controllers at all. Why not just chuck everything in one place (either Model or Controller). But it's a fundamental decision that will effect how easy it is to maintain and debug your finished application so it needs thinking about carefully. I often start with UML diagrams to layout the data structures and Use Cases that I expect to use and then translate that into the code.

Your question:
1) if you always want stipslashes to be used on the data, then do it in the Model
2) checking data for validity. If you want to provide feedback to the user then it's best to do it in the Controller, but sometimes only the Model understands what is valid. If a user provides an invalid email, then I would check in the Controller as that can just reload the View with an error message. I would try to structure the system so that validity is checked as soon as possible, normally in the Controller.

I look at it this way. The Model classes handle the data, they provide data structures (either arrays or objects) for the rest of the system to work with. As such, they should provide a consistent interface to the data, so in the case of using stripslashes, then that would be part of the data Model.

Lets say you are dealing with Customers. The Customers Model class would provide a standard interface for the rest of the system to operate on Customers. For example, get(), save(), delete(), get_all() and so on. Customers->get($id) would either return an array of customer information or (my preference) a Customer object.

Your Controller classes will handle the interactions between the Models and the Views. So you may have a view that displays Customer details. Your Controller will get a Customer object from the Customers model, and maybe order details from the Orders model, possibly Payment details from the Payment model, check that it is consistent and then pass what is needed off to the View page to be viewed (or edited etc) by the user.

The MVC architecture was (and is) an attempt to manage complexity in large, complex, software systems by layering the architecture and encapsulating the storage, application logic and display. Whether you need it or not in a simpler system may not be important, but in the long term it is worth getting to understand the benefits and how to get it work for you.

HTH
#3

[eluser]mvdg27[/eluser]
Hi,

I've been testing out some stuff tomorrow, and came up with the following setup:

Code:
function save_section() {

        // validate user input
        $errors = $this->website_model->validate_section_data();

        // if no errors were encountered
        if(!count($errors)) {
            try {
                if(!$this->website_model->save_section_data($this->area_db, $this->section_id))
                    throw new Exception('save_section_error');
                
                $o['success'] = true;
                                
            }
            catch (Exception $e) {
                $o['success'] = false;
                $o['exception'] = $e->getMessage();
            }
        }
        else {
            $o['success'] = false;
            $o['errors'] = $errors;            
        }

        print json_encode($o);
                        
    }

So I created a seperate function to check the user input in my website_model, which returns ther errors if any. From the website_model I call general functions in 'general_model' like:

Code:
function update_section($table, $section_id, $db_input) {
        try {
            // does table exists    
            if(!$this->db->table_exists($table))
                throw new Exception('table_does_not_exist');
            
            // set modifications
            $db_input['modified'] = date('Y-m-d H:i:s');
            $db_input['modifier'] = $this->session->userdata('user_id');
        
            // update the table
            $this->db->where('id', $section_id);
            $this->db->update($table, $db_input);
            
            // check if rows were updated
            if(!$this->db->affected_rows())
                throw new Exception('no_rows_updated');
            
            return true;
        }
        catch (Exception $e) {
            return false;
        }                
    }

This seems to work very nice .. it already made my code a lot cleaner! What do you guys think of this setup?

-Michiel
#4

[eluser]nikefido[/eluser]
Looking good so far!

To expand on jalalski's point:

You can try to make your code modular. Basically, this means keep methods/functions related to one "subject" in one area. You might want to create a validation model or you might want to keep validation specific to one process in one model with other related methods (hopes that makes sense).

One other side note: If you can keep from using the Try Catch usage vs simple returning TRUE (or some data) and FALSE, you should favor the latter.

Try/Catch and throwing exceptions are expensive operations in terms of processing.

Try/Catch blocks are best used in areas where you don't have full control over responses/code.

This can include 3rd party libraries where you don't want to have to hack into the code to see whats going on (many libraries are written so all you need to know if a simpler front-end interface, and learning other people's complex code is a time drain).

This can also include things like web services, where you cannot control the data being sent back.

In you're code above, if you don't throw an exception but rather a flag that an error occurred, you then open up avenues to pass back better error messages that front-end users can read and understand.

Just my 2cents - Hope you find this useful.
#5

[eluser]mkhairul[/eluser]
Hi Michiel!

I've been using CI for quite a while and after some trial and error. I decided to use Models just to retrieve the needed data from the database and usually the model name will be the table name.

I created a template to be used in Komodo Edit to create a model. Here's an example.
Code:
class [[%w:else:]]_model extends Model {

    function [[%w:else:]]_model()
    {
        parent::Model();
        $this->table_name = strtolower('[[%w:else:]]');
        
        if(!$this->db->table_exists($this->table_name))
        {
            $CI =& get_instance();
            $CI->load->dbforge();
            $CI->dbforge->add_key('id', TRUE);
            $fields = array(
                'id' => array(
                                    'type' => 'INT',
                                    'constraint' => 11,
                                    'unsigned' => TRUE,
                                    'auto_increment' => TRUE
                                  ),
                'name' => array(
                                    'type' => 'VARCHAR',
                                    'constraint' => '255',
                                    'null' => FALSE,
                                  ),
                'uid' => array(
                                    'type' => 'INT',
                                    'constraint' => 11,
                                    'unsigned' => TRUE,
                                  ),
                'timecreated' => array(
                                    'type' => 'INT',
                                    'constraint' => 11,
                                    'null' => FALSE,
                                  )
                
            );
            $CI->dbforge->add_field($fields);
            
            if ($CI->dbforge->create_table($this->table_name))
            {
                log_message('debug', ucfirst($this->table_name) . " Table Created.");
            }
    }
    }
    
    function delete($id)
    {
        $this->db->where('id', $id);
        $this->db->delete($this->table_name);
        return TRUE;
    }

    function get_details($id)
    {
        $this->db->where('id', $id);
        $query = $this->db->get($this->table_name);
        return ($query->num_rows() > 0) ? $query->row():FALSE;
    }
    
    function get_list()
    {
        $query = $this->db->get($this->table_name);
        return ($query->num_rows() > 0) ? $query:FALSE;
    }
    
    function get_name($id)
    {
        if(!$id){ return FALSE; }
        
        $this->db->where('id', $id);
        $query = $this->db->get($this->table_name);
        if($query->num_rows() > 0)
        {
            $result = $query->row();
            return $result->name;
        }
        else
        {
            return FALSE;
        }
    }
    
    function insert($data)
    {
        $this->db->set($data);
        $this->db->insert($this->table_name);
        
        return $this->db->insert_id();
    }
    
    function update($id, $data)
    {
        $this->db->where('id', $id);
        $this->db->set($data);
        $this->db->update($this->table_name);
    }
}
The [[%w:else:]] will be replaced with whatever you highlight in a new tab.

For the controller, I usually stay clear of putting any HTML inside it. It will be a nightmare to maintain it later on. I use controllers to manage the flow of data from/to libraries and models. The data retrieving, checking, I leave it up to the library.
#6

[eluser]mvdg27[/eluser]
Hi guys! Thanks for the replies, really usefull!

@nikefido: Your remark about making the code modular is exactly what I'm after. And I think I'm achieving this already. While re-coding my second controller to using models, I figured out I could use many functionality I had used in my 'website_model'. So I guess now it's just a matter of putting the right functionality in the right model. But now at least I have the feeling I'm on the right track.

With respect to your remark about Try Catch usage .. this is actually the first I'm using this. An experiment so to say. I really like the coding style, but if I also get your point about using this on remote libraries. Especially if this makes my script slower, it's probably wise to change it back to if-else. Do you have any number to back your statement up?

@mkhairul: Nice example!! Thanks, this will be really useful for me and other as well.

-Michiel
#7

[eluser]jalalski[/eluser]
I don't think you need 'get_instance()' in the constructor. It already is an instance.
#8

[eluser]nikefido[/eluser]
[quote author="jalalski" date="1231770074"]I don't think you need 'get_instance()' in the constructor. It already is an instance.[/quote]

I've actually been running into (what I assume is a) bug where I had to use get_instance in my model - my custom library classes were being created simply using $this->load->('whatever'); but then I couldn't access them via $this->whatever->....(Received the running a method on a non-object error).
#9

[eluser]dmorin[/eluser]
Interesting blog post that might help: http://blog.astrumfutura.com/archives/37...iated.html

CI takes a very loose stance on models and while this is very helpful for getting started, it's also good to realize their full potential (not just db access!!) to create more maintainable applications.
#10

[eluser]jalalski[/eluser]
Nikefido: I haven't run into that, although I did sometimes get strange errors when I used a name that conflicted with something internal to CI.




Theme © iAndrew 2016 - Forum software by © MyBB