Link to home
Start Free TrialLog in
Avatar of Crazy Horse
Crazy HorseFlag for South Africa

asked on

Creating my own form validation class

I have found that for forms on the website I am building, I keep writing the same validation code over and over again. So, I thought it might be a good idea to create a validation class. I am still learning OOP so please bear with me.

I must also mention that I am using a custom mini MVC framework.

Just to see if I could get it working,  I put 2 methods into my class:

class Validation {
	

	public function val_email($email)
	{
		if(filter_var($email, FILTER_VALIDATE_EMAIL)) {
			return true;
		}
	}
	
	public function is_empty($formval)
	{
		if(!empty(trim($formval))) {
			return true;
		}
	}
}

Open in new window


I then instantiate it in my controller :

      
public function test()
	{
		if($_SERVER['REQUEST_METHOD'] == 'POST') {
			
		$email = filter_var($_POST['email'], FILTER_SANITIZE_EMAIL);
		$name = filter_var($_POST['name'], FILTER_SANITIZE_STRING);
			
		$data = [
			
			'email' => $email,
			'name' => $name,
			'message' => ''
		];
		
		
		$validation = new Validation;
			
		if(!$validation->val_email($email)) {
			
			$data['message'] .= 'Invalid email address <br />';
		}
	
		if(!$validation->is_empty($name)) {
			
			$data['message'] .= 'Name required <br />';
		}
			
		if(!empty($data['message'])) {
			
			$this->view('pages/test', $data);
			
		} else {
			
			die('okay');
		}
		
		$data = [
			
			'email' => $_POST['email'],
			'message' => ''
		];
		
		$this->view('pages/test', $data);
			
		}
		
		else {
			
			$data = [
				
				'email' => '',
				'name' => '',
				'message' => ''
			];
			
			$this->view('pages/test', $data);
		}
	}

Open in new window


This works in that if I submit the form, I get the validation errors until I fill out the form correctly. But I just wanted to know if what I am doing is correct. Just because it works doesn't mean it's right!

I do have another question as well though, it doesn't seem to work if I do the opposite and return false which is what I would prefer to do actually.

	public function is_empty($formval)
	{
		if(empty(trim($formval))) {
			
			return false;
		}
	}

Open in new window

Avatar of Julian Hansen
Julian Hansen
Flag of South Africa image

But I just wanted to know if what I am doing is correct. Just because it works doesn't mean it's right!
Difficult to answer.

What I can tell you is that there are some standard ways of doing this. One practice is to use a rule based system where rules are specified in an Array - you specify the rule you want to run and the data it must run it against.

The problem I have found with these approaches is they work well in simple cases but tend to be a bit more complicated when you have situations where more complex validation.
For instance, if user clicks checkbox A then field B must have a value otherwise it can be empty - to give an example.

To accommodate such scenarios the solutions tend to become overly complicated trying to cater for every eventuality - to the point that the code you have to write for them is almost more complicated than if you were just to write the validation rules directly.

Most coders are lazy - if they have done something before they don't want to have to do it again so there is a natural tendency to want to write generic code - but one needs to keep in mind the problem you are trying to solve - writing the best generic validation code when you need to check an email address and name field are completed might not be the most optimal solution in terms of time and effort.

So, long story short, start out simple - use some simple validation routines that will validate various types of data and then build the code for your form that combines these using whatever other logic your form requires (for example cross dependence).
Avatar of Crazy Horse

ASKER

Thanks, Julian. I wanted to use this for the more simple validation that I do over and over again, like for example validating an email address or name for example. I would't use it for a once off unique validation for a specific project. I jsut want to use it for more generic stuff. And it might have been nice to get the validation class to also output the actual error.
Or are you saying I shouldn't have a validation class?
SOLUTION
Avatar of gr8gonzo
gr8gonzo
Flag of United States of America image

Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
Thanks for that gr8gonzo, I will try this out when I get home from work later today.
gr8gonzo, after reading you post a few times I think I am more confused than when I started, haha :)

I want to just take baby steps here as that might make things easier for me.

Firstly, I am having trouble with the actual class. Validation only fails if I make the method return true, e.g.:

	public static function val_email($email)
	{
		if(filter_var($email, FILTER_VALIDATE_EMAIL)) {
			
				return true;
		}
	}

Open in new window


Then in the controller I have:

$validation_errors = array();

if(!Validation::val_email($email))
			
		{
		  $validation_errors[] = 'Invalid email address';
		}

		if(count($validation_errors)) {
		
		$data['message'] = implode("<br/>", $validation_errors);
		
		}

                // if validation fails, send $data array to the view
		if(!empty($data['message'])) {
			
			$this->view('pages/test', $data);
			
		} else {
			
			die('okay');
		}

Open in new window


That works okay. I get the validation error if I don't input a proper email address. Why does this not work if I make the method return false? That to me seems to make sense. If something returns false, show an error. But I can't seem to get it to work like that.

If I do this:

      
public static function val_email($email)
	{
		if(filter_var($email, FILTER_VALIDATE_EMAIL) === false) {
			
				return false;
		}
	}

Open in new window


Then the validation error displays whether I enter a valid email address or not.
ASKER CERTIFIED SOLUTION
Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
The problem with your validation is that you are not returning a true when it's successful:

      public static function val_email($email)
      {
            if(filter_var($email, FILTER_VALIDATE_EMAIL) === false) {
                        return false;
            }
      }

So in a VALID email scenario, the function won't return ANYTHING (which basically means it's returning a null). So if you take this line:

if(!Validation::val_email($email))

and think about the return values in each scenario, it's basically the equivalent of this:

INVALID EMAIL:
if(false == false)

VALID EMAIL:
if(null == false)

PHP is very loose with some concepts, especially when comparing data types, and tries to handle a lot of situations for you, and sometimes it does a bad job. This is a good example of that.

PHP believes that null == false.
PHP also believes that 0 == false.
PHP also believes that false == false.

It's essentially trying to make assumptions about what you meant instead of just throwing a warning or an error message.

So the solution here is to return true to your function if it doesn't return false:

      public static function val_email($email)
      {
            if(filter_var($email, FILTER_VALIDATE_EMAIL) === false) {
                        return false;
            }
           else
            {
                        return true;
            }
     }
Thanks to you both for your posts which are very comprehensive. But if you have the patience to bear with me, I want to just break this down into smaller pieces as the main goal of this isn't for me to actually build a validation class. It has been said on this thread and I have read in a few different places around the net that creating a validation class is a bad idea, and I have also seen suggestions that using value objects would be better ( I still have to look this up as I don't know what that is).

Anyway, my main objective here is to get the class to interact with my controller, if that is the right way of doing this. So, in my validation class, all I have is this:

class Validation {
	
	
      public function val_email($email)
      {
            if(filter_var($email, FILTER_VALIDATE_EMAIL) === false) {
				
            return false;
				
	  } else {
				
             return true;
				
            }
     }
}

Open in new window


I am going to leave out the static bit for now as I really want to go as basic as possible here just to grasp it and then I can play around with it and make it more complex and so on.

In my controller I have just instantiated the class:

$validation = new Validation;

	if(!$validation->val_email($email)) {
			
		$data['message'] = 'Invalid email address';
	}

		if(!empty($data['message'])) {
			
                        // send errors to the view
			$this->view('pages/test', $data);
			
		} else {
			
                        //successful validation
			die('okay');
		}

Open in new window


After reading your posts, it makes more sense to put the errors into an array, but again, I am just trying to make sure I understand the creating a class, instantiating it and then using the created object correctly.

So, my focus is mainly around this code:

	if(!$validation->val_email($email)) {
			
		$data['message'] = 'Invalid email address';
	}

Open in new window


I apologise if this is a really contrived example, but I need to grasp the very basics of this in order to see the bigger picture.
Your example looks fine - is there a specific problem at this point with that new code?
@ gr8gonzo, no problem at this point. I am just glad that the way I have done it is correct, even if it is pointless. I just wanted to get the principle right in terms of creating the class and instantiating it in the controller.

I would have to expand on it to work like your and Julian's example for it to be worthwhile. But, my last question before I close this question out is, should I bother trying to start a validation class and building it up over time for convenience  on simple validation like email, names, dates etc. or should I just keep validating  in the controller as I have been up until now?

Eg:

      
	if(empty($name)) {
			
			$data['message'] .= 'Name is required <br />';
		}
			
		if(filter_var($email, FILTER_VALIDATE_EMAIL) === false) {
			
			$data['message'] .= 'Invalid email address <br />';
		}
		
		if(!empty($data['message'])) {
			
			$this->view('pages/test', $data);
			
		} else {
			
			die('okay');
		}

Open in new window

Personally, I think a validation class has a lot of merit. It eliminates the chances of small mistakes when you're copying validation logic around.

If you're striving for as pure of an MVC model as possible, then really ALL of the validation should be done by models. The controller should really just start the validation call on the model and get back the results. Controllers are not the correct place for input validation logic.

That said, I tend to see MVC as a "take-what-you-need" type of system where you try to adhere as close as possible but make exceptions as necessary. You really have to think about how it might change in the future and be maintained over time, and decide for yourself if it's worth setting up different models for each validation scenario.
Interesting. I am new to MVC but thought you would want to validate your data before sending it to the model. My models are currently strictly database transactions.
Take a look at my last post - it is a complete validation system that you can use almost out of the box (and extend if you need).
Thanks Julian, I will just have to try get it to work in my current MVC structure and figure out which part should be the class etc. But I suppose that I should also be careful not to change things for the sake of it. But that is one of the things I find most confusing about php. There are a lot of ways to do things and I already have something that works. But that's the catch, just because something works doesn't mean it is the right way to do it! So, I don't want to get used to doing something and it's bad practice or wrong etc. Still a lot to learn! Thanks guys for your valuable input. I will go through all of this and try implement into my existing framework.
Thanks for both of your time, it is much appreciated.