HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Service class that handles form validations

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
handlesservicethatvalidationsformclass

Problem

I've made a (service) class to handle all form validation of my application. It contains a set of frequently used validation callables extracted into private methods that I use to build each is[FormName]Valid(); method. It would probably end up containing validation logic for about 5 or 6 forms of which it contains 2 already. So the class could become relatively big; roughly about 230 lines.

It works completely fine and was the best solution I could think of. But since this would pretty much be considered a god object, at some point I would want to adapt to a better pattern. I've got some clues that my class might be somewhat extraordinary, because I haven't seen anyone else do it like this before.

Are there any better / more efficient solutions?

```
namespace Model\Application;

use Library\Validator;
use Model\Infrastructure\Mapper\UserMapper;

class FormValidationService
{
private $validator;
private $userMapper;
private $errors;

public function __construct(Validator $validator, UserMapper $userMapper)
{
$this->validator = $validator;
$this->userMapper = $userMapper;
}

public function isLoginValid($email, $password)
{
$this->validator->reset();

$this->validator->isValid('email', [
$this->notEmpty($email),
$this->email($email),
]);

$this->validator->isValid('password', [
$this->notEmpty($password),
$this->minChars($password, 6)
]);

return !$this->errors['Login'] = $this->validator->findErrors();
}

public function isSignUpValid($firstName, $lastName, $email, $confirmationEmail, $password, $terms)
{
$this->validator->reset();

$this->validator->isValid('firstName', [
$this->notEmpty($firstName),
$this->maxChars($firstName, 35)
]);

$this->validator->isValid('lastName', [
$this->notEmpty($lastName),
$this->maxChars($lastNam

Solution

Are you sure it's a form?

Everytime I see this 'form-validation' code I shrug. A form shouldn't be validated. Maybe on client side for better UX. But not on serverside.

Now before you burn with those torches and send me to hell, let me explain:

A form is client-side input mechanism that helps the user to interact with your application. Instead of having ot manually perform POST/PUT/DELETE requests, we give him some nice UX sugar. For instance a html ` element.

The form is however not the only way to interact with the application. If it is a GET form we can easily perform that in the browser with
yourapp.tld?email=no-form@wow.tld. l33t hack0rs will even be able to do the same for POST/PUT/... requests.

So yes we need validation. But not form validation.

Validate where you should

Most application these days have some sort of Router that dispatches a Request to a Controller. For instance a
Authentication@logMeIn Controller/method. The controller will try and authenticate a user given the credentials provided.

If no credentials are send with the request, then the controller has no idea what to do. Otherwise he does:
* Ask the Authentication service if the credentials are correct
* authenticate the user + return success || return error

All validation is done by the Authentication service. That Authentication service will have an instance of a Validator that it will use. Something like this:

function areTheCredentialsCorrect($username, $password)
{
    if ( !$this->validator->isEmail($username) ) {
        throw new InvalidArgumentException('I need an email!');
    }

    $user = $this->repository->getUser('email','=',$username);

    return password_verify($user->password, $password);
}


The authentication service validates or the username is of the correct format. Here we need an email.
We could even add exra logic sugar for 2 cases. If $username is an email -> get user by email otherwise get it by 'oldskool_username'.

Same goes for creation of a user. Our repository that handles the creation of users will check or the passed in arguments are correct:

function createUser($name, $email, $password)
{
    if ( !$this->validator->isAlpha($name) ) {
        //we only allow alpha-characters. We don't like the O'lastname guys
        throw new InvalidArgumentException('No scottisch people allowed!');
    }

    if ( $this->valdiator->isEmail($email) ) {
        throw new InvalidArgumentException('I need an email!');
    }

    if ( !$this->validator->isPassword($password) ) {
        throw new InvalidArgumentException('Strong passwords only please');
    }

    $this->database->create($name, $email, $password);
}


This not only gives us cleaner code. It also decouples the logic used in your html form (name stuff) from the validator. A validator shouldn't care how your forms are named/build. It should only care about validating data that is inserted into it. Doesn't matter from where it comes.

So be gone with your FormValidator. Simply let the code that handles the data itself perform validation. this way you have less security bugs of the kind where you forgot to validate one little input that then is directly printed into your sql. Don't give bobby tables a chance ;)

EDIT:
Different meanings of a Form are used here. Having methods like
isSignUpValid and isLoginValid means you will eventually have a method for every possible action with input. If your app get's bigger you will have tons of methods. and everytime you add a form you will need a different method, copy-pasting most of the inner workings.

As Elias correctly points out, once the data is put into your Models the valiation should have happened. But not in some kind of all knowing
FormValidationService`.

And yes, you will probably have some generic testing:

  • is the specified REQUEST_METHOD valid?



  • can I answer in the correct ACCEPT language (text/html, JSON, ...)



  • SRP



  • spam blocking



  • ...



Let the more specific validation checks happen in the controller:

  • is the email unique



  • is the password valid



  • is the firstName of an accepted length

Code Snippets

function areTheCredentialsCorrect($username, $password)
{
    if ( !$this->validator->isEmail($username) ) {
        throw new InvalidArgumentException('I need an email!');
    }

    $user = $this->repository->getUser('email','=',$username);

    return password_verify($user->password, $password);
}
function createUser($name, $email, $password)
{
    if ( !$this->validator->isAlpha($name) ) {
        //we only allow alpha-characters. We don't like the O'lastname guys
        throw new InvalidArgumentException('No scottisch people allowed!');
    }

    if ( $this->valdiator->isEmail($email) ) {
        throw new InvalidArgumentException('I need an email!');
    }

    if ( !$this->validator->isPassword($password) ) {
        throw new InvalidArgumentException('Strong passwords only please');
    }

    $this->database->create($name, $email, $password);
}

Context

StackExchange Code Review Q#61767, answer score: 4

Revisions (0)

No revisions yet.