3

i am starting to put validation/sanitization in my codeigniter models, and before diving in too deep I am looking for some suggestions on best practices. the form validation library for controllers is great, but obviously i don't want to rely on the controllers to send me good data.

currently I return bool values, TRUE (or data) on success, FALSE on failure, which makes it really hard to pass error messages back to the caller. I would like to get away from the FALSE on failure.

while definitely not an expert, i have started reading quite a bit on Exceptions and have come across them quite a bit with external libraries, and they seem like a good candidate for this. my question is, is this appropriate use of exceptions? are model errors exceptional errors?

a possible example:

<?php
class person_model extends CI_Model{

    public function getPersonById($personId){

        //check for int
        if(!is_int($personId) OR $personId < 0){
            throw new Exception('Invalid person ID');
        }

        //setup query
        $this->db->select('*')
            ->where('personId', $personId);

        //run query
        $result = $this->db->get('person');

        //failed to get
        if(!$result){
            throw new Exception('DB query failed');
            //should i also return false?
            return FALSE;
        }

        //got info
        else{
            return $result;
        }
    }
}
?>

thanks for the help!

EDIT:

I have to say I am quite surprised by the responses suggesting that data validation should only be done in the controller. Models are the last barrier to your data storage. The model is the data and the rules applying to that data, your application logic. Data validation seems like application logic to me. Also you may have many controllers accessing the same model method. Do you want to rely on both controllers implementing the same validation? That seems silly to me.

Also, not all data is coming from user input, some of it could be hardcoded into the script by your programmer writing the controller. What if they pass a string when your model is expecting an integer? Or pass an incorrectly formatted date? shouldn't the model say something about that.

I'm open to a discussion, but I feel data validation DEFINITELY belongs in the model. (in addition to the controller, and even the view (html5/javascript for convenience))

Andrew Brown
  • 5,330
  • 3
  • 22
  • 39
  • 1
    your models should not contain validation, leave this to the controllers, and refer to the models for clean data. – Jakub Dec 10 '13 at 20:48
  • Andrew, your models should be clean data, it needs to be validated before the model. The model is your data, it is not your logic, controller handles that. – Jakub Dec 11 '13 at 15:25
  • you said: "yes, i use those extensively in my controllers, but they are not built for models, since they look for POST data" Thats not quite true since you can use the CI form validation in a model - it works exactly the same way. this->input->post is also available in the model. – cartalot Dec 12 '13 at 02:00
  • 1
    POST may be available to models but that seems like a very bad idea. you are losing your whole separation of responsibilities. models should only know what the controller tells them. they should be unaware of the user interface. by referencing post data in your model, you are tying it to only post data, rather than allowing controllers to pass in data from what could be multiple sources. – Andrew Brown Dec 12 '13 at 20:00
  • ha well this is why its "fat controllers" vs "thin controllers" -- people have different preferences. and some people prefer libraries over models. personally i look at the form validation as "gritty data details" which i prefer to have tucked away in a model. for example if you add a new field to a contact form - you don't have to change any code in a thin controller, just in the relevant model. also consider that you must have all the field names in the model for the database actions. why repeat all that in the controller? – cartalot Dec 12 '13 at 20:24

4 Answers4

1

the easiest way i've found to deal with this is to always check for the negative condition first with an if check. this also makes it easy to check multiple steps.

always return something back from your model methods whenever possible. Like even in this example - we need to validate the form - i would use codeigniters form validation to validate that its an integer etc. if it passes validation then we need the $personId for the database search. So instead of just returning true/false from the validation - if validation passes then return the $personId : 

function getperson() {
 // Validate the form 
 // AND if its valid, return the validated $personId 
 // Note the separate private method if the validation failed 
 if ( ! $personId = $this->person->_validateGetPersonForm() ) {
     $this->error_msg = 'Error in validating the form. Please use a number.';
 $this->_showGetPersonFailed() ; }

 elseif ( ! $person = $this->person->getBy($personId)  ) {
$this->error_msg = 'There is no person in our records with the number:'. $personId;
$this->_showGetPersonFailed() ; }

 else {  $this->_showResultsFor($person) ; } 
  }

$this->error_msg is automatically available for any of your views and because we have broken out validation and the database search - the error message is easily appropriate for the exact condition. if the search failed then there is a private method _showGetPersonFailed() to show the form again. the last if / else is usually success - there is a separate private method to handle showing the results with the appropriate views.

also suggest not using the word "model" in the file name like "person_model". it just clutters up the overall naming and forces you to type the word model over and over again :-) think of it this way: if you are calling something from a controller and getting back results - its almost always going to be a model. and all your model files will always be in a folder named models.

cartalot
  • 3,147
  • 1
  • 16
  • 14
  • the problem you can run into with CI is naming conflicts with controllers and models. this is why it was a suggestion i had read to append _model to all models. – Andrew Brown Dec 11 '13 at 03:34
  • one controller can call many models. models do not call controllers. so there is not going to be any naming conflicts. its more of a crutch when you are first learning. i used to name my views "contactform_view" for the same reason. just don't name a folder the same thing as a file. for example if you have a folder named "admin" in your controller folder with a bunch of admin controllers, you can't also have a controller named "admin". – cartalot Dec 12 '13 at 01:52
0

Go to:

application/config/database.php

and search for db_debug to be TRUE. For example:

...
$db['default']['dbprefix'] = '';
$db['default']['pconnect'] = TRUE;
$db['default']['db_debug'] = TRUE; //<-- Have this to true
$db['default']['cache_on'] = FALSE;
$db['default']['cachedir'] = '';
...
John Skoumbourdis
  • 3,041
  • 28
  • 34
  • Then this is just a duplicated question to: http://stackoverflow.com/questions/6217786/codeigniter-and-throwing-exceptions – John Skoumbourdis Dec 10 '13 at 20:49
  • umm John read the question, the OP wants to know how to do validation in his models and if exceptions are the way to go. – Jakub Dec 10 '13 at 20:53
  • thanks for the suggestions, I had read that question beforehand but wasn't really what i was looking for. – Andrew Brown Dec 11 '13 at 03:37
0

In my models, I never throw exceptions or errors. I always return false on a query that returns null. If there is an SQL error in your code you will be notified by codeigniter automatically. I would stay away from throwing exceptions if your query returns no results. You can then handle the query if it returns false with your controller.

EDIT: Also you should check the data being queried into the database in the controller, and not the model. In my opinion the model should be used strictly for querying data, not for error/data checking, you can do this before hand in the controller when it is submitted.

An example:

Model

function search_replies($term){
    $this->db->like('ticket_id', $term);
    $this->db->or_like('reply_from', $term);
    $this->db->or_like('reply_from_name', $term);
    $this->db->or_like('reply_content', $term);

    $query = $this->db->get($this->table_ticket_replies);

    // Returns the result if the number of rows is greater than 0, returns false otherwise        
    if ($query->num_rows() > 0) return $query->result();
    return false;
}

Controller

function example_controller(){
    if($this->search_model->search_replies('Test')){
        $data['results'] = $this->search_model->search_replies('Test');
    }

    $this->load->view('search_results', $data);
}

View

<?php 
if(isset($results)){
    echo 'Retrieved Results';
    foreach($results as $result){
    }
} else{
    ?>
    <h2>No results for search term!</h2>
    <?php
}
?>
Steve Bauman
  • 8,165
  • 7
  • 40
  • 56
0

You have probably missed out this part of the form validation page in the user guide:

Showing Errors Individually

Usage:

echo form_error('field_name');

Just put in whatever field name you are using in place of "field_name", for example "username" or "email" etc.

Naveed Hasan
  • 641
  • 7
  • 19
  • he also missed that validation is done in the controller, not the model. – Jakub Dec 11 '13 at 00:28
  • 1
    Well, depends... if you take a look at My_Model in PyroCMS, validations are done in this core class which is extended by models defined by us. However, you are correct, we should not show the errors through the models, that should be left to the controllers. – Naveed Hasan Dec 11 '13 at 00:35
  • i am aware of the form_error and such, but again those are relating to the `views` and indirectly the controller form validation, which I already use. my question is in regard to Model validation. – Andrew Brown Dec 11 '13 at 03:39
  • Have you tried using validation rules? using `$this->form_validation->set_rules();`? – Naveed Hasan Dec 11 '13 at 03:46
  • yes, i use those extensively in my controllers, but they are not built for models, since they look for POST data – Andrew Brown Dec 11 '13 at 18:00