patternphpMinor
Validation and Form classes
Viewed 0 times
andformvalidationclasses
Problem
I have written
I stored in
What do you think of
Validation Class
```
class Validation
{
protected $fields = array();
private $errors = array();
public function setRules($field, $errorMessage, $rules = array())
{
if (count($_POST) == 0) {
throw new Exception("The array of post parameters is empty");
}
if ($field == '') {
throw new Exception("field parameter is empty");
}
if (!is_array($rules) || count($rules) == 0) {
throw new Exception("The array of rules parameter is empty");
}
$errorMessage = ($errorMessage == '') ? $field : $errorMessage;
$this->fields[] = array(
'name' => $field,
'errorMessage' => $errorMessage,
'rules' => $rules,
);
}
public function validate()
{
if (count($_POST) == 0) {
throw new Exception("The array of post parameters is empty");
}
if (count($this->fields) == 0) {
throw new Exception("Validation rules is not set");
}
foreach ($this->fields as $field) {
$fieldName = $field['name'];
if (isset($_POST[$fieldName])) {
foreach ($field['rules'] as $rule) {
$param = false;
if ($arr = explode('=', $rule)) {
if (isset($arr[0]) && isset($arr[1])) {
$param = $arr[1];
$rule = $arr[0];
}
Validation and Form classes in PHP.Validationclass allow you to define the rules of each field and a error message.
Formclass allow you to get a error message and also allow you to get a value of POST via$_SESSION.
I stored in
$_SESSION because if validation did not pass it will need to redirect back to main Form page and repopulate all the fields using value() method. What do you think of
Validation and Form classes - what can be improved?Validation Class
```
class Validation
{
protected $fields = array();
private $errors = array();
public function setRules($field, $errorMessage, $rules = array())
{
if (count($_POST) == 0) {
throw new Exception("The array of post parameters is empty");
}
if ($field == '') {
throw new Exception("field parameter is empty");
}
if (!is_array($rules) || count($rules) == 0) {
throw new Exception("The array of rules parameter is empty");
}
$errorMessage = ($errorMessage == '') ? $field : $errorMessage;
$this->fields[] = array(
'name' => $field,
'errorMessage' => $errorMessage,
'rules' => $rules,
);
}
public function validate()
{
if (count($_POST) == 0) {
throw new Exception("The array of post parameters is empty");
}
if (count($this->fields) == 0) {
throw new Exception("Validation rules is not set");
}
foreach ($this->fields as $field) {
$fieldName = $field['name'];
if (isset($_POST[$fieldName])) {
foreach ($field['rules'] as $rule) {
$param = false;
if ($arr = explode('=', $rule)) {
if (isset($arr[0]) && isset($arr[1])) {
$param = $arr[1];
$rule = $arr[0];
}
Solution
Short on time, but here's a few quick things:
(I will likely come back to this. Also, I haven't forgotten about your post from 2 days ago that I said the same thing on... Just been very busy :D)
- Consider making individual validators rather than putting all of the different validation rules in one class.
- One option would be to make a
Fieldclass and haveFormcontain a collection ofFields (with each field having a collection ofValidators)
- Pass in $_POST instead of accessing it in your class (what if you want to validate $_GET or something?)
- Form shouldn't directly depend on
$_SESSION
$_SESSION['posts'] = $_POST;The form shouldn't be responsible for storing state. If you want to do that, do it outside of theFormclass.
- How do you know which validation(s) failed?
- The two exceptions thrown in validate should be reconsidered. A missing field should probably be considered empty (and thus a required validation failed), and a
Formwith no rules, should probably always validate astrue. (Also, they should probably be specialized exception classes, not just the genericException)
return ($value != '') ? true : false;Is redundant:return ($val != '');
- In particular, any expression of the form
(expr) ? true : falsecan be written as(expr)
- Be careful with implicit typecasts (for example, some of your string comparisons might should be
===)
$this->errors[$fieldName] = array('errorMessage' => $field['errorMessage']);You're clobbering any old errors
- There should probably be some way to tell which elements have errors. How did you know name has an error if validate fails? How do you know it's not email?
(I will likely come back to this. Also, I haven't forgotten about your post from 2 days ago that I said the same thing on... Just been very busy :D)
Context
StackExchange Code Review Q#16018, answer score: 5
Revisions (0)
No revisions yet.