Avoiding many if blocks for validation checking

There are cases that we want to validate input data before we send them to business logic layer for processing, computations etc. This validation, in most cases, is done in isolation or it might include some cross-checking with external data or other inputs. Take a look at the following example that validates user input for registration data.

public void register(String email, String name, int age) {
 String EMAIL_PATTERN = 
 "^[_A-Za-z0-9-\\+]+(\\.[_A-Za-z0-9-]+)*@"
 + "[A-Za-z0-9-]+(\\.[A-Za-z0-9]+)*(\\.[A-Za-z]{2,})$";
 Pattern pattern = Pattern.compile(EMAIL_PATTERN);
 
 List<String> forbiddenDomains = Arrays.asList("domain1", "domain2");
 if ( email == null || email.trim().equals("")){
   throw new IllegalArgumentException("Email should not be empty!");
 }
 
 if ( !pattern.matcher(email).matches()) {
   throw new IllegalArgumentException("Email is not a valid email!");
 }
 
 if ( forbiddenDomains.contains(email)){
   throw new IllegalArgumentException("Email belongs to a forbidden email");
 }
 
 if ( name == null || name.trim().equals("")){
   throw new IllegalArgumentException("Name should not be empty!");
 }
if ( !name.matches("[a-zA-Z]+")){
   throw new IllegalArgumentException("Name should contain only characters");
 }
 if ( age <= 18){
   throw new IllegalArgumentException("Age should be greater than 18");
 }
// More code to do the actual registration
}

The cyclomatic complexity of this method is really high and it might get worse if there are more fields to validate or if we add the actual business logic. Of course we can split the code in two private methods ( validate, doRegister ) but the problem with several if blocks will be moved to the private methods. Besides this method is doing more than one thing and is hard to test. When I ask junior developers to refactor this code and make it more readable, testable and maintainable they look at me like an alien :”How am I supposed to make it simpler. How can I replace these if blocks?” Well here’s a solution that works fine, honors the Single Responsibility Pattern and makes the code easier to read.

To better understand the solution, think each of these if blocks as a validation rule. Now it’s time to model these rules.

First create an interface with one method. In Java 8 terms, it’s called a functional interface, like the following.

public interface RegistrationRule{
 void validate();
}

Now it’s time to transform each validation check to a registration rule. But before we do that we need to address a small issue. Our interface implementation should be able to handle registration data but as you see we have different types of data. So what we need here is to encapsulate registration data in a single object like this :

public class RegistrationData{
 private String name;
 private String email;
 private int age;
// Setters - Getters to follow
}

Now we can improve our functional interface:

public interface RegistrationRule{
void validate(RegistrationData regData);
}

and start writing our rule set. For instance let’s try to implement the email validation.

public class EmailValidatationRule implements RegistrationRule{
 private static final String EMAIL_PATTERN = 
 "^[_A-Za-z0-9-\\+]+(\\.[_A-Za-z0-9-]+)*@"
 + "[A-Za-z0-9-]+(\\.[A-Za-z0-9]+)*(\\.[A-Za-z]{2,})$";
 private final Pattern pattern = Pattern.compile(EMAIL_PATTERN);
 @Override
 public void validate(RegistrationData regData) {
 if ( !pattern.matcher(regData.email).matches()) {
   throw new IllegalArgumentException("Email is not a valid email!");
 }
}

It’s clear that we have isolated in the above class the email validation. We can do the same for all rules of our initial implementation. Now we can re-write our register method to use the validation rules.

 List<RegistrationRule> rules = new ArrayList<>();
 rules.add(new EmailValidatationRule());
 rules.add(new EmailEmptinessRule());
 rules.add(new ForbiddenEmailDomainsRule());
 rules.add(new NameEmptinessRule());
 rules.add(new AlphabeticNameRule());
 
 for ( RegistrationRule rule : rules){
  rule.validate(regData);
 }

To make it even better we can create a Rules class using the Factory pattern and a static method get() that will return the list of rules. And our final implementation will look like this

for ( RegistrationRule rule : Rules.get()){
  rule.validate(regData);
}

Comparing the initial version of our register method to the final one leaves room for doubts. Our new version is more compact, more readable and of course more testable. The actual checks have been moved to separate classes (which are easy to test also) and all methods do only one thing ( try to always keep that in mind ).

ArtOfSoftwareGardening_mini

If you found this post useful please don’t forget to share it and if you have some spare time take a look at my upcoming book about the “Art of Software Gardening”

Related Posts

4 comments

Hey,

Great example of single responsibility.

My only concern is RegistrationData, where you store all the data for all the rules, where only one
item is neccessary for each rule.

Also, wouldn’t it be better to make it an inner class of the RegistrationRule interface ? This way these 2 entities would be naturally closer, as they are strictly related.

Sth like this:

interface RegistrationRule {
void validate(final RegistrationData regData);

final class RegistrationData {
T t;
}
}

Best regards,
Jacek

Sorry, <T> where cut out, they are meant to be parametrized types of course 😉

Hi Jack
Thanks for your feedback. Yes the RegistrationRule could be an inner interface or place it in the same package with package level access. About the first comment, indeed in my example only one item is needed each time. But what about the type of each data. Then things are getting a little more complicated in the interface definition. And if we have a rule that involves more than one RegistrationData attributes? Even worse 🙂
So maybe passing the whole data set is simpler and cover all cases.

Since this nice and simple post, hints for a good way on solving a very common problem, I could not resist to add a small addition on the above example, mostly related with the validation and the propagation of errors back to the called rather than the initial point of avoiding null checks.

So my addition is related to the use of Exceptions for controlling flow or logic versus the use of a simple result or validation POJO that would be ‘returned’ to the caller.

So my simple ValidationResult would look like this (you may add any extra features in a real case scenarion)

public class ValidationResult {
private boolean hasErrors=false;
private String validationErrorMsg;
…..
}

Then the interface would be something like

@Override
public ValidationResult validate(RegistrationData regData) {
ValidationResult result =new ValidationResult();
if ( !pattern.matcher(regData.email).matches()) {
result.setHasErrors(true);
result.setValidationErrorMsg(“Email is not a valid email!”)
}
return result;

And then the rules running code would be
List results = new ArrayList();
for ( RegistrationRule rule : Rules.get()){
ValidationResult aResult= rule.validate(regData);
results.add(aResult)
}

or you could add only the results with errors e.g
ValidationResult aResult= rule.validate(regData);
if (aResult.getHasErrors){
results.add(aResult)
}

That’s all, the implementation details may vary, I just wanted to note it as a point rather than providing a full implementation.

Keep it up Patrokle great work 🙂

Leave a reply