3
\$\begingroup\$

Let's say I have this User model:

class User
{
    public $id;
    public $email;
    public $password;
    public $errors;

    public function isValid()
    {
        if (strpos($this->email, '@') === false) {
            $this->errors['email'] = 'Please enter an email address';
        }
        if (!$this->password) {
            $this->errors['password'] = 'Please enter a password';
        } elseif (strlen($this->password) < 4) {
            $this->errors['password'] = 'Please enter a longer password';
        }

        return !$this->errors;
    }
}

And let's say I have this UserDAO for retrieving, adding, updating, and deleting users:

class UserDAO
{
    protected $conn;
    protected $logger;

    public function __construct(PDO $dbh, Logger $logger) 
    {
        $this->dbh = $dbh;
        $this->logger = $logger;
    }

    public function getUsers() 
    {
        $rows = null;
        try {
            $rows = $this->dbh->query("SELECT * FROM users")->fetchAll();
        } catch (PDOException $e) {
            $this->logger->log($e->getMessage(), __METHOD__);
        }
        return $rows;
    }

    public function getUserById($id) 
    {
        $row = null;
        try {
            $sth = $this->dbh->prepare("SELECT * FROM users WHERE id = ?");
            $sth->execute(array($id));
            $row = $sth->fetchObject('User');
        } catch (PDOException $e) {
            $this->logger->log($e->getMessage(), __METHOD__);
        }
        return $row;
    }

    public function addUser(User &$user) 
    {
        $success = false;
        try {
            $sth = $this->dbh->prepare("
                INSERT INTO users (email, password) VALUES (?, ?)
            ");
            $sth->execute(array($user->email, $user->password));
            if ($success = (bool) $sth->rowCount()) {
                $user->id = $this->dbh->lastInsertId();
            }
        } catch (PDOException $e) {
            $this->logger->log($e->getMessage(), __METHOD__);
        }
        return $success;
    }

    public function updateUser(User $user) 
    {
        // ...
    }

    public function deleteUser($id) 
    {
        // ...
    }

    public function isEmailUnique($email) 
    {
        $count = 0;
        try {
            $sth = $this->dbh->prepare("SELECT COUNT(id) FROM users WHERE email = LOWER(?)");
            $sth->execute(array($email));
            $count = $sth->fetchColumn();
        } catch (PDOException $e) {
            $this->logger->log($e->getMessage(), __METHOD__);
        }
        return !$count;
    }
}

When I process a form, I typically do something like this:

// ...

$userDAO = new UserDAO($dbh, $logger);
$user = new User();
$user->email = filter_input(INPUT_POST, 'email', FILTER_VALIDATE_EMAIL);
$user->password = filter_input(INPUT_POST, 'password');
// validate user
if ($user->isValid()) {
    // check if email address is unique (SO UGLY!)
    if ($userDAO->isEmailUnique($user->email)) {
        $user->errors['email'] = 'Please use a different email address';
    }
    // save user
    if ($user->addUser($user)) {
        // ...
    } else {
        // ...
    }
} else {
    // do something with $user->errors
}

This works great for simple cases, but how can I improve this? In particular, how can I do validations that require database access? For example, if I wanted to check that the email is unique and make it part of my validation, what would be a better way of doing this?

\$\endgroup\$

1 Answer 1

2
+50
\$\begingroup\$

Best approach would be to have another Object to do the validation based on specific configuration.

Your User should NOT be responsible for validating itself, neither should the DAO. Validation is a specific task and should be handled by a specific Class.

Example of this approach would be something like this (omitiong interfaces for brevity):

validation class

Class Validator 
{
   protected $errors = [];

   public function __construct(DaoInterface $daoDependency, $otherDependency)
   {
      //...
   }

   public function validate($object, array $rules) 
   {
       foreach ($rules as $rule)
       {
           $propertyName = $rule['property'];
           if (!$this->isValidateProperty($object, $propertyName, $rule['config'])) {
               $this->errors[$object->$propertyName] = 'is invalid ' . $rule['message'];
           }
       }
   }

   public function isValid()
   {
       return (bool)count($this->errors);
   }

   public function getErrors()
   {
       return $this->errors;
   }

   protected function isValidProperty($object, $propertyName, $ruleConfig)
   {
       // $ruleConfig should have information about what validation you want to use and relevant validation should be executed base on it (it can separate objects etc)
       if (!property_exists($object, $propertyName)
          throw new MissingPropertyException();


       if (isset($ruleConfig['email-unique-in-db') {
           if ($daoDependecny->emailExists($object->$propertyName) {
               return false;
           }
       }
       if (isset($ruleConfig['email-is-valid') {
           return (bool)strpos($object->$propertyName, '@');               
       }
       // ... etc but better with objects for each validation type.
   }
}

usage

$userDAO = new UserDAO($dbh, $logger);
$user = new User();
$user->email = filter_input(INPUT_POST, 'email', FILTER_VALIDATE_EMAIL);
$user->password = filter_input(INPUT_POST, 'password');

// get your validation rules
// you can have them stored in other object, plain array, yml whatever you like. for instance $rules = new UserConstrains(); or
$rules['config] = [
  'email' => [
      'email-unique-in-db',
      'email-is-valid'
  ],
  'otherfield' => [
      'callback' => ['someCallbackFunction']
  ]
];

// create validator
$validator = new Validator($userDao, $otherDepenency);
$validator->validate($user, $rules);
// validate user

if (validator->isValid()) {
   // .. do stuff
} else {
   // do something with $user->errors
}

The separation of concerns is the most important thing here. You can extend/decorate the validator class for specific use cases

Also if would recommend using some specialized validation library for this task for instance symfony2 validation component can be used stand alone: http://symfony.com/doc/current/book/validation.html

\$\endgroup\$
2
  • \$\begingroup\$ Thanks, that is a good solution to move the validation. Symfony2 validation component will probably do the job but I am always reluctant with using validation libraries like this or GUMP. I always feel that they are too much for simple things. \$\endgroup\$
    – Mikey
    Commented Feb 19, 2016 at 19:25
  • \$\begingroup\$ sometimes they can be, but in my opinion most of the time spent on implementing and learning such library, will be shorter than combined time of gradually implementing it and hitting unseen bugs. As well with something bigger like sf2, you got quite a nice support group of people when some problem arise. \$\endgroup\$
    – nakashu
    Commented Feb 19, 2016 at 21:19

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.