Is the Spring Validator mechanism as friendly as it could be?

When I first came across the Spring validator classes in October 2006, I thought at the time that they weren’t particular clear. They seem fit into that realm of programming where everyone just copies what other people have done without understanding what’s going on and it all works, which is how I’ve seem them used. I guess that today it’d be called Cargo Cult programming. As you know from my previous blog, ValidationUtils will validate your target object without any obvious reference to it, which to me is confusing. You also know that you can pass null in to the Validator.validate() method for the target object, but again there’s no explanation why.

On reflection, several factors seemed to have influenced the design of this mechanism which included the fact that it was written before generics and annotations, it was written to support the now deprecated MVC classes such as SimpleFormController and the guys at Spring wanted to make it simple to use (in which they succeeded).

Errors in the case of ValidationUtils is an IN OUT parameter type, with implicit knowledge of the state of its related target object and can update its own state based on tests on the target’s state by other objects. Which brings me to my major criticism: this idea isn’t at all bad, its just that the Errors interface is badly named.

The name reflects an object that holds information about lots of errors, rather than an object that’s got data that the collaborating ValidationUtils class can test and then store. It’s really both the target object and the errors result object all rolled into one and that’s not reflected in the name. I’ve tried thinking of a better name, but one didn’t quickly come to mind, which to me is an indication that either I don’t really understand what’s going on, or any class which is implementing this interface is breaking the Single Responsibility Principal. The best rename I could quickly come up with is ErrorsContext, but it isn’t that good as Context is a weasel word for a class name, because of its vagueness.

By the way, the answer as to why you can pass null in to the Validator.validate() method for the target object is that the ValidationUtils class doesn’t use it as the Errors class already has a copy of the target object's state data. The target object parameter is only used for your own domain’s business rule validation.

Having criticised the Spring validation mechanism, I thought that I’d suggest a few improvements that could be achieved with a little refactoring.

Firstly, it would be a good idea to update the Validate interface to use generics, in which case it’ll look really simple:

public interface Validator<T> {

  /**
   * @param targetObject The target object to validate
   * @param errors contextual state about the validation process (never null)
   */
  public void validate(T targetObject, Errors errors);
}

Secondly, I’d prefer to see the ValidationUtils class use separate target and errors objects. This could be achieved by making the ValidationUtils a real object rather than a static helper (I’ve always mistrusted static helpers) and by adding a static factory method:

    ValidationUtils<Address> validationUtils = ValidationUtils.getInstance(targetObject,
        errors);

    validationUtils.rejectIfEmpty("post_code", "postcode.required",
        "The postcode is a required field");

This may add an extra line of code to your validator, but it does separate the target object and errors context responsibilities: the target stores the target’s state and the error object holds the error context for that target.

Note, the code above implies that since we’ve added generics to the Validator interface, we’ll be adding it to all other interfaces for compile time type safety which will improve code stability.

Having made these small changes, and using the Address validation scenario in yesterday’s blog, then the AddressValidator would now look like this:

public class AddressValidator implements Validator<Address> {

  /**
   * @param obj The target object to validate
   * @param errors contextual state about the validation process (never null)
   */
  @Override
  public void validate(Address targetObject, Errors errors) {

    commonValidation(targetObject, errors);
    businessValidation(targetObject, errors);
  }

  /** Use ValidationUtils to perform common validation tasks */
  private void commonValidation(Address targetObject, Errors errors) {

    ValidationUtils<Address> validationUtils = ValidationUtils.getInstance(targetObject,
        errors);

    validationUtils.rejectIfEmpty("post_code", "postcode.required",
        "The postcode is a required field");
    // TODO Validate other fields using the ValidationUtils static class.
  }

  /** More specific validation rules not handled by ValidationUtils */
  private void businessValidation(Object obj, Errors errors) {

    Address address = (Address) obj;
    String postCode = address.getPost_code();
    if (isNotNull(postCode) && isNotBirminghamPostCode(postCode)) {
      errors.rejectValue("post_code", "not.birmingham.postcode",
          "Post code should be Birmingham and begin with a letter 'B'");
    }
  }

  private boolean isNotNull(String postCode) {

    return postCode != null;
  }

  /** The first character of the Birmingham post code is 'B' */
  private boolean isNotBirminghamPostCode(String postCode) {

    char val = postCode.charAt(0);
    return val != 'B';
  }
}

There are many other changes that could be suggested (as with any code), so this has really been just an exercise in how to critic some code in order to better understand what’s going on. My guess is that the main client for the Validator interface is the deprecated BaseCommandController, which means that the interface is also obsolete and only available for backwards compatibility.

As the old MVC class have bee deprecated in Spring 3.0, then it’s high time I looked at the new annotation based classes. More on that later.