Model now doing data/form validation. How to retur

2019-06-07 20:35发布

问题:

Since today I have started validating form data in my Model layer instead of in my Controllers. I am going to shorten the code snippets as much as possible.

This is a method from my User domain object (setLastName() method is basically the same)

public function setFirstName($firstName) {

    if(!$firstName) throw new \InvalidArgumentException('Some message');
    if( strlen($firstName) < 2  || strlen($firstName) > 20 ) throw new \LengthException('Some message');
    if(preg_match('/[^a-zA-Z\'.-\s]/', $firstName)) throw new FormatException('Some message');

    $this->firstName = $firstName;
}

In my Controller I have something like this

$userService = $this->serviceFactory->build('User');

try {
    $userService->register('John', 'M');
} 
catch(\InvalidArgumentException $ex) {

}
catch(\LengthException $ex) {

}
catch(etc etc)

In my UserService method register() I have something like

$user->setFirstName($firstName);
$user->setLastName($lastName);

When running the setFirstName() method it will successfully set the supplied first name. The setLastName() method will throw a LengthException as it is too short.

That is what I want but when that comes back to the service layer and then to the controller and I catch it I know that a LengthException was thrown but I can't give the user a proper message like "The supplied last name was too short" because I do not know for which field the exception was thrown, just the type of exception.

How do I get around this? Thanks.

回答1:

View instances should be requesting information from the model layer. Controllers is not responsible for passing the information.

This would also mean that you obsessive-compulsive use of exceptions, that cause your abstraction layers to leak, would be completely pointless. "Error" is just a state of model layer. It is an expected situation, not an exception.

Controllers in MVC are responsible for changing the state of model layer and (quite rarely) the state of current view instance. They should not be receiving any feedback from services.



回答2:

Instead of returning some message, why not return a useful error message such as "supplied First name is too short". This can then be returned to the user.

Alternatively you can see that when extending exceptions you can specify additional information such as numeric codes - you could of course use this.

Or of course you can create a subclass of Exception for difference circumstances, but you could end up with hundreds of Exception subclasses which would of course be messy.



回答3:

I have the same question. I think that most of the people who say that all the validation should be done in the model never developed a full PHP MVC application themselves and only know books and theory. Never a piece of code is seen about that topic.

Anyway I have thought of a possible solution. What do you think about the code below:

// Controller

$user = User::make(
    $_POST['lastname'], $_POST['firstname'], 
    $_POST['gender'], [...]
);
if(is_array($user)) {
    // store the errors in a view variable and forward, or store in session and redirect
    $_SESSION['errors'] = $user;
    $this->_redirect('add');
    exit;
}

// Model

public static make($lastname, $firstname, $gender, [...]) {
    $errors = array();
    if(/* test firstname */) $errors[] = 'model_error_firstname';
    if(/* test lastname */) $errors[] = 'model_error_lastname';
    if(!empty($errors)) return $errors;

    return new User($lastname, $firstname, $gender, [...]);
}

The model would have a static function that will return either an array with errors in case something went wrong, or a new model object if validation was ok.

In your controller you test whether an array was returned or not.

Maybe I would put the constructor of the User as private, because if you build a user with the constructor directly, you would skip all the validations. But this does not mean that it becomes a Singleton.

Maybe I would also sanitize the form fields and make them safe before passing them to the model.

The keys like model_error_xyz would be found in a translation file with the appropriate text.

Update:

Actually I think that you could just throw a custom exception from the constructor and that contains an array of messages. But the reason that I didn't propose that, is that it leads to half-constructed objects, at least in Java for example, but hey, PHP is not Java...

You would also have to validate each setter function :( it seems tedious to do validation in the model instance.

Any thoughts are welcome.