MVC Architecture – Should Models Handle Validation?

Architecturemvc

I am trying to re-architect a web application I developed to use the MVC pattern, but I'm not sure if validation should be handled in the model or not. For example, I'm setting up one of my models like this:

class AM_Products extends AM_Object 
{
    public function save( $new_data = array() ) 
    {
        // Save code
    }
}

First Question: So I'm wondering if my save method should call a validation function on $new_data or assume that the data has already been validated?

Also, if it were to offer validation, I'm thinking some of the model code to define data types would look like this:

class AM_Products extends AM_Object
{
    protected function init() // Called by __construct in AM_Object
    {
        // This would match up to the database column `age`
        register_property( 'age', 'Age', array( 'type' => 'int', 'min' => 10, 'max' => 30 ) ); 
    }
}

Second Question: Every child class of AM_Object would run register_property for each column in the database of that specific object. I'm not sure if this is a good way of doing it or not.

Third Question: If validation should be handled by the model, should it return an error message or an error code and have the view use the code to display an appropriate message?

Best Answer

First Answer: A key role of the model is to maintain integrity. However processing user input is a responsibility of a controller.

That is, the controller must translate user data (which most of the time is just strings) into something meaningful. This requires parsing (and may depend on such things as the locale, given that for example, there are different decimal operators etc.).
So the actual validation, as in "is the data well formed?", should be performed by the controller. However the verification, as in "does the data make sense?" should be performed within the model.

To clarify this with an example:
Assume your application allows you to add some entities, with a date (an issue with a dead-line for example). You might have an API, where dates might be represented as mere Unix time stamps, while when coming from a HTML page, it will be a set of different values or a string in the format of MM/DD/YYYY. You don't want this information in the model. You want each controller to individually try to figure out the date. However, when the date is then passed to the model, the model must maintain integrity. For example, it might make sense to not allow dates in the past, or dates, that are on holidays/sundays, etc.

Your controller contains input (processing) rules. Your model contains business rules. You want your business rules to always be enforced, no matter what happens. Assuming you had business rules in the controller, then you'd have to duplicate them, should you ever create a different controller.

Second Answer: The approach does make sense, however the method could be made more powerful. Instead of the last parameter being an array, it should be an instance of IContstraint which is defined as:

interface IConstraint {
     function test($value);//returns bool
}

And for numbers you could have something as

class NumConstraint {
    var $grain;
    var $min;
    var $max;
    function __construct($grain = 1, $min = NULL, $max = NULL) {
         if ($min === NULL) $min = INT_MIN;
         if ($max === NULL) $max = INT_MAX;
         $this->min = $min;
         $this->max = $max;
         $this->grain = $grain;
    }
    function test($value) {
         return ($value % $this->grain == 0 && $value >= $min && $value <= $max);
    }
}

Also I don't see what 'Age' is meant to represent, to be honest. Is it the actual property name? Assuming there's a convention by default, the parameter could simple go to the end of the function and be optional. If not set, it would default to the to_camel_case of the DB column name.

Thus the example call would look like:

register_property('age', new NumConstraint(1, 10, 30));

The point of using interfaces is that you can add more and more constraints as you go and they can be as complicated as you want. For a string to match a regular expression. For a date to be at least 7 days ahead. And so on.

Third Answer: Every Model entity should have a method like Result checkValue(string property, mixed value). The controller should call it prior to setting data. The Result should have all the information about whether the check failed, and in case it did, give reasons, so the controller can propagate those to the view accordingly.
If a wrong value is passed to the model, the model should simply respond by raising an exception.

Related Topic