views:

97

answers:

4

Currently, I create an object and use its setters to set the get/post data received from client. And call the validate() before calling the save() function like this:

//member registration
$m=new Member();
$m->setName($_POST['name']);
$m->setBirthDate($_POST['birthdate']);
$m->setAddress($_POST['address']);

$arrOfErrMsgs=$m->validate();

if(!empty($arrOfErrMsgs)){
//echo some error messages to client or redirect to a page that shows the error
exit();
}

$saveSuccess=$m->save(); //to be safe, inside this save function, it will also call validate() again before saving, so that even someone forgot to call validate() before calling save() by mistake, no dirty data will appear in the database 

if($saveSuccess){
//echo a success message to client or redirect to a success page

}else{
//echo save failed message to client (normally this should not happen unless db server suddenly fails)
}

exit();

While this works, but there should be some alternatives. For example, maybe some of the validations can be done in the setters.

I would like to know what patterns for validation in model in PHP are the most common?

+2  A: 

The controller should validate, the model shouldn't handle any non-trusted data.

erenon
I think you are wrong! The model should do the logic (in this case the validation) the controller should just redirect the user to where he or she should go!
AntonioCS
+1  A: 

I think this is a perfectly fine way to perform business validation. Perhaps your save() method could return a string with the error message if the data is invalid, else true (saving the redundant validate() call). This is the approach I currently take - although it really depends on how you want to design your API. As long as it's predictable and makes sense.

Like erenon commented, the model shouldn't be performing low-level validation like checking for ints, date formats, malicious code, etc. - these belong elsewhere (e.g. Zend_Form). However for business validation like member must be over 21 years old, I don't see a problem.

Richard Nguyen
+2  A: 

I don't recommend this way of validating your models.

First off, have the controller validate the input, in so far as insuring that it's a string when it should be, an integer when it should be and so on. But NOT the business logic.

When you create a new Member, I suggest passing all the information in through the constructor instead. That way the constructor can check the OVERALL model consistency, while the setter methods should have consistency checks for each datum that they are responsible for.

For instance, if you had another piece of information like firstHeardOf (the first time they heard of whatever site they're becoming a member of), the setter should ensure that it's a valid date and it's AFTER the date of birth, at the very least. This creates an ordering issue of methods, and requiring methods be called in a particular order isn't good practice, instead the constructor can be used to guarantee the ordering of the calls.

The save method is now just a simple method, it saves the data, and really why should it have any other responsibility. At most it should double check that the constructed member is still valid, which is where you might want to pull the constructor validation code into a private/protected validation method and call that in both places.

A quick word on responsibilities, and how they break down here, at least in my mind:

  • The constructor is responsible for creating a valid/properly initialized Member object
  • The setters are responsible for setting valid data within the member object for the datum
  • The save method is responsible for saving a valid member object
Saem
+1  A: 
<?php

abstract class Model {

    protected $errors = array();

    public function validate() {
     // Implement validation logic, overriden in subclasses
    }

    public function isValid() {
     $this->validate();
     return empty($this->errors);
    }

    public function save() {
     if (!$this->isValid()) {
      return false;
     }

     // Perform the saving operation
    }
}

class Member extends Model {

    // setters and getters and constructor

    public function validate() {
     if (empty($this->name)) {
      $this->errors[] = 'name can not be empty';
     }
    }
}

$member = new Member(array(
    'name' => 'Hanse'
));

if ($member->save()) {
    // Print success messages
} else {
    // Print error messages
    print_r($member->getErrors());
}

I prefer an approach as that of the code above. Gives you a clean and flexible API.

Hanse