The best approach to validate fields

Posted on

Problem

I wonder if there is a better approach to validate the object fields. For specific types of objects some fields cannot be empty:

public class CredentialsValidator {

  public boolean validateCredentialsRequest(ValidationCredentialsRequest credentialsRequest) {
    switch (credentialsRequest.getCredentialType()) {
      case TYPEA:
        return validatetypeACredentials(credentialsRequest);
      case TYPEB:
        return validateTypeBCredentials(credentialsRequest);
      case TYPEC:
        return validateTypeCCredentials(credentialsRequest);
    }
    return true;
  }

  private boolean validateTypeCCredentials(ValidationCredentialsRequest credentials) {
    if (credentials.getIsNewCredential()) {
      return nonNull(credentials.getUsername())
          && !credentials.getUsername().isEmpty()
          && nonNull(credentials.getPassword())
          && !credentials.getPassword().isEmpty();
    }
    return nonNull(credentials.getCredentialsId());
  }

  private boolean validateTypeBCredentials(ValidationCredentialsRequest credentials) {
    if (credentials.getIsNewCredential()) {
      return nonNull(credentials.getHostname())
          && !credentials.getHostname().isEmpty()
          && nonNull(credentials.getPort())
          && !credentials.getPort().isEmpty()
          && nonNull(credentials.getUsername())
          && !credentials.getUsername().isEmpty()
          && nonNull(credentials.getPassword())
          && !credentials.getPassword().isEmpty();
    }
    return nonNull(credentials.getCredentialsId())
        && nonNull(credentials.getHostname())
        && !credentials.getHostname().isEmpty()
        && nonNull(credentials.getPort())
        && !credentials.getPort().isEmpty();
  }

  private boolean validatetypeACredentials(ValidationCredentialsRequest credentials) {
    if (credentials.getIsNewCredential()) {
      return (nonNull(credentials.getDomain())
              && !credentials.getDomain().isEmpty()
              && nonNull(credentials.getUsername())
              && !credentials.getUsername().isEmpty()
              && nonNull(credentials.getPassword())
              && !credentials.getPassword().isEmpty())
          || (nonNull(credentials.getSharePoint().getUsername())
              && !credentials.getSharePoint().getUsername().isEmpty()
              && nonNull(credentials.getSharePoint().getPassword())
              && !credentials.getSharePoint().getPassword().isEmpty());
    }
    return nonNull(credentials.getCredentialsId())
        && nonNull(credentials.getDomain())
        && !credentials.getDomain().isEmpty();
  }

My main class that invokes Validator:

public Mono<ValidationCredentialsResponse> validateCredentials(
      Mono<ValidationCredentialsRequest> validationCredentials) {
    return validationCredentials
        .filter(credentialsValidator::validateCredentialsRequest)
        .switchIfEmpty(Mono.error(new InvalidCredentialsException(MESSAGE)))
        .flatMap(
            validCredentials ->
                securityService


I use Spring Boot and Webflux for reactive programming

Solution

From an object-oriented point of view there is a better way you can handle this scenario.

Feature Envy

Currently the code snipped has a Code Smell that is called Feature Envy:

The whole point of objects is that they are a technique to package data with the processes used on that data. A classic [code] smell is a method that seems more interested in a class other than the one it is in. The most common focus of the envy is the data.

— Martin Fowler, Refactoring: Improving the Design of Existing Code, Chap. 3, p. 80

The class CredentialsValidator only interacts with data from ValidationCredentialsRequest by accessing it via all the getters:

nonNull(credentials.getUsername())
&& !credentials.getHostname().isEmpty()
/* ... */

Instead, this logic could be in ValidationCredentialsRequest, and CredentialsValidator only interacts with the validation methods instead of the getters:

public boolean validateCredentialsRequest(ValidationCredentialsRequest credentialsRequest) {
   switch (credentialsRequest.getCredentialType()) {
       case TYPEA:
           return credentialsRequest.isTypeAValid();
       case TYPEB:
           return credentialsRequest.isTypeBValid();
       case TYPEC:
           return credentialsRequest.isTypeCValid();
   }
   return true;
}
class ValidationCredentialsRequest {
   /* ... */

   public boolean isTypeAValid() {/* ... */ }

   public boolean isTypeBValid() {/* ... */ }

   public boolean isTypeCValid() {
       if (newCredential) {
           return nonNull(username)
               && !username().isEmpty()
               && nonNull(password)
               && !password.isEmpty();
       }
       return nonNull(credentialsId);
   }

}

Polymorphism

Polymorphism is a concept where one thing can be something else at the same time: for example, a Shape can be a Rectangle at the same time.

With Polymorphism we can achieve that we no longer need the switch case. This would have the advantage that we don’t have to change the CredentialsValidator class when adding new validation logic:

class CredentialsValidator {
    
    public boolean validateCredentialsRequest(ValidationCredentialsRequest credentialsRequest) {
        return credentialsRequest.isValid();
    }
    
}

From here all different “shapes” of ValidationCredentialsRequest need to share the same interface – all need the isValid method:

abstract class ValidationCredentialsRequest {
    /* ... */
    abstract boolen isValid();
}

class TypeARequest extends ValidationCredentialsRequest {
    /* ... */
    public boolean isValid() { /* ... */ }
}

class TypeBRequest extends ValidationCredentialsRequest {
    /* ... */
    public boolean isValid() { /* ... */ }
}

class TypeCRequest extends ValidationCredentialsRequest {
    /* ... */
    public boolean isValid() {
        if (newCredential) {
            return nonNull(username)
                && !username().isEmpty()
                && nonNull(password)
                && !password.isEmpty();
        }
        return nonNull(credentialsId);
    }
}

Now when a new kind of ValidationCredentialsRequest is needed, we just have to create a new “form” of it, and it works without changing the CredentialsValidator.

Edit

Use in Main

In the code for the main you provided nothing needs to be changed.

But if you would like to add the Polymorphism to your code, you need to change it where you create the ValidationCredentialsRequest objects. There you would create the different “shapes” of ValidationCredentialsRequest instead of ValidationCredentialsRequest itself.

Leave a Reply

Your email address will not be published. Required fields are marked *