Lost access to same property trying to use best practice / design pattern

Question

I had:

• in a class implementing Validator:
  • an $errormessages property
  • an isCorrect() method

In the isCorrect method, I had:

switch ($type): 
    case 'email':
        isEmailCorrect();
    case 'password':
        isPasswordCorrect();
    case 'x':
        isXCorrect();

isEmailCorrect(), isPasswordCorrect() and isXCorrect() had access to the same property with all error messages

Now, I have:

• in Validator:
  • an $errormessages property
• in an EmailValidator class extending Validator:
  • an isCorrect() method
• in a PasswordValidator class extending Validator:
  • an isCorrect() method
• in a XValidator class extending Validator:
  • an isCorrect() method

Now, in a file calling the isCorrect() methods, I have:

$EmailValidator = new EmailValidator();
$PasswordValidator = new PasswordValidator();
$XValidator = new XValidator();

$EmailValidator->isCorrect(), $PasswordValidator->isCorrect() and $XValidator->isCorrect() don't have access to the same property with all error messages

$errormessages are in different instances of different classes. They should be one, but are three.

What now?

Answer 1

Well you could make an external properties factory to control access to your property, assuming you are talking about properties files that is the approach I usually take.

If you are referring to a shared field then you can place it in your base class and access it that way.

Answer 2

I'll often use Zend_Validate classes to perform the validation, and aggregate any error message to a property on the object that's being validated (as well as a flag that control valid status).

My setup would be similar to this:

class User {
  public $email;
  protected $_errorMessages = array();

  public function validate()
  {
    $valid = true;

    $emailValidator = new EmailValidator();
    if (!$emailValidator->isCorrect($this->email)) {
      $valid = false;

      // validation message are added to the $errormessages property in
      // the validator class upon failure of isCorrect()
      $this->_errorMessages[] = $emailValidator->getMessages();
    }

    // repeat this for all your validators

    return $valid
  }

  public function getErrorMessages()
  {
     return $this->_errorMessages();
  }
}

// in your page....
if (!$user->validate()) {
  $messages = $user->getErrorMessages();
}

Answer 3

I think you should develop another class: a ValidatorChain, which takes an arbitrary amount of validators, and that aggregates the errormessages of all validators that it has tested

For reference see the docs on Zend Framework's Validator Chain

EDIT

Now that I re-evaluate your question (thanks to Bryan M's comment); why do you want each individual Validator to have access to other Validators' error messages? I would say that collecting each individual Validators' error messages is the responsibility of an object higher in the hierarchy.

If, however, you want individual Validators to be able to act based on context, in other words, based on what the results of other Validators are, then I suppose you could add a $context parameter to the isCorrect method. This could for instance accept an arbitrary amount of Validators or something similar.

Something like:

interface ValidatorInterface
{
    public function isCorrect( array $context );
    public function getMessages();
}

abstract class ValidatorContextOptions
{
    const SHOULD_BE_PRESENT = 'shouldBePresent';
    const SHOULD_NOT_BE_PRESENT = 'shouldNotBePresent';
    const SHOULD_BE_VALID = 'shouldBeValid';
}

class EmailValidator implements ValidatorInterface
{
    protected $_field;

    protected $_contextOptions = array();

    protected $_messages = array();

    public function __construct( $field, array $contextOptions )
    {
        $this->_field = $field;
        $this->_contextOptions = $contextOptions;
    }

    public function isCorrect( array $context = null )
    {
        foreach( $this->_contextOptions as $field => $options )
        {
            foreach( $options as $option )
            {
               switch( $option )
               {
                   case ValidatorContextOptions::SHOULD_NOT_BE_PRESENT:
                       if( isset( $context[ $field ] )
                           && $context[ $field ] instanceof ValidatorInterface )
                       {
                           $this->_messages[] = $field . ' should not be present';
                           return false;
                       }
                       break;
                   case ValidatorContextOptions::SHOULD_BE_PRESENT:
                       if( !isset( $context[ $field ] )
                           || !$context[ $field ] instanceof ValidatorInterface )
                       {
                           $this->_messages[] = $field . ' should be present';
                           return false;
                       }
                       break;
                   case ValidatorContextOptions::SHOULD_BE_VALID:
                       if( !isset( $context[ $field ] )
                           || !$context[ $field ] instanceof ValidatorInterface
                           || !$context[ $field ]->isCorrect() )
                       {
                           $this->_messages[] = $field . ' should be valid';
                           return false;
                       }
                       break;
               }
            }
        }

        // some dummy function which you should replace with real validation
        return isAnEmailAddress( $this->_field );
    }

    public function getMessages()
    {
        return $this->_messages;
    }
}

Usage:

$emailValidatorContextOptions = array
(
    'phone' => array( 
                   ValidatorContextOptions::SHOULD_BE_PRESENT,
                   ValidatorContextOptions::SHOULD_BE_VALID
               )
);

$phoneValidator = new PhoneValidator( $phoneString );
$emailValidator = new EmailValidator( $emailString, $emailValidatorContextOptions );

if( !$emailValidator->isCorrect( array( 'phone' => $phoneValidator ) ) )
{
    print_r( $emailValidator->getMessages() );
}

What I've shown here, needs a lot more thinking (and I really mean A LOT), is buggy as hell and definately not bulletproof. But I hope you catch my drift of where I'm going with this.

Moreover, where do you insert the values in your validator that need to be validated anyway?

Answer 4

If I read you right, you want multiple instances to share the same error messages property, such that you can instantiate several validators and have them all contribute to a single array.

If this is the case, there are a few ways to do it. One would be to create a validator manager class which has responsibility for instantiating and registering validators. Then once validation is complete you could call $validator_manager->getErrors() which would aggregate the errors present in all the validators registered with it.

Another way you could do it would be to use a singleton error store class, which you acquire in the constructor of each validator. Each validator's addError() method would then delegate the job to the singleton.

There are other methods still, but basically you're going to have to use another object, either for managing the validators or storing the errors.

Answer 5

Someone below mentioned using a singleton for this.

I am not convinced that it's a great use of that design pattern, especially since it's commonly held that singletons are the "anti-pattern" and often over/mis-used.

Nonetheless, keeping that in mind, here's an example along those lines:

<?php

//Error Class implemented as a Singleton
class ErrorClass
{

  static private $instance = false;
  static private $errorMessages;

  function getInstance() {
    if (!self::$instance) {
      self::$instance = new ErrorClass();
      self::$errorMessages = "No errors;";
    }
    return self::$instance;
  }

  public function setError($errorMessage){
    self::$instance->errorMessages .= $errorMessage;
  }

  public function getError(){
    return self::$instance->errorMessages;
  }

}

abstract class AbstractClass
{

  // Force Extending class to define this method
  abstract protected function isCorrect($b);

  // Common Method for setting error
  public function setError($errorMessage) {
    ErrorClass::getInstance()->setError($errorMessage);   
  }

  // Common Method for getting error
  public function getError() {
    return ErrorClass::getInstance()->getError();   
  }
}

class EmailValidator extends AbstractClass
{

  public function isCorrect($b) {
    if(!$b) {
      $this->setError('EmailValidator->isCorrect();');
    }
  }

}

class PasswordValidator extends AbstractClass
{

  public function isCorrect($b) {
    if(!$b) {
      $this->setError('PasswordValidator->isCorrect();');
    }
  }

}

// Then in your code
$errorState = 1; // used for testing purposes

$EmailValidator = new EmailValidator(); 
$EmailValidator->isCorrect($errorState);

$PasswordValidator = new PasswordValidator(); 
$PasswordValidator->isCorrect($errorState);

echo $EmailValidator->getError();
echo $PasswordValidator->getError();