0

Want to reduce code from these validations, these validators' classes verify and return if inputs are valid or invalid, it's a reduction, I will validate some panels and almost 40 fields. Want to see if there is some pattern to simplify this, code is more than 300 lines which I believe to be a bad practice.

package Validation1;

import java.util.regex.Matcher;
import java.util.regex.Pattern;

public class MinimalReproducibleExampleValidation {

public static void main(String[] args) {

    boolean saveToDatabase = true;

    String name = "Richard";
    String idCard = "123456789";
    String address = "Main Street 454";

    Entity entity = new Entity ();

    /// Name Validation
    if (saveToDatabase) {
        ValidationEntity nameValidation = new 
            ValidationEntity(ValidationEntity.Regex.Alphabetic, 
                name, "ID Card", 0, 13);
        saveToDatabase = nameValidation.isValid();
        entity.setName(name);
    }
    
    /// ID Card Validation
    if (saveToDatabase) {
        ValidationEntity idCardValidator = new 
            ValidationEntity(ValidationEntity.Regex.Numerical, 
                idCard, "ID Card", 0, 13);
        saveToDatabase = idCardValidator.isValid();
        entity.setIdCard(idCard);
    }
    
    /// EMail Validation
    if (saveToDatabase) {
        ValidationEntity emailValidator = new 
            ValidationEntity(ValidationEntity.Regex.AlphaNumerical, 
                address, "Address", 1, 9);
        saveToDatabase = emailValidator.isValid();
        entity.setAddress(address);
    }
    // If every field is valid, save
    if (saveToDatabase) {
        new EntityDao().save(entity);
    }
}
}

and:

class ValidationEntity {

    private Regex regex;
    private String input;
    private String errorMessage;
    private Integer minimum;
    private Integer maximum;

public ValidationEntity(Regex regex, String input, String errorMessage, int minimum, int maximum) {
    this.regex = regex;
    this.input = input;
    this.errorMessage = errorMessage;
    this.minimum = minimum;
    this.maximum = maximum;
}

public boolean isValid() {
    Pattern pattern = Pattern.compile(getRegexFormat(), Pattern.CASE_INSENSITIVE);
    Matcher matcher = pattern.matcher(input);
    return matcher.matches();
}

   public String getRegexFormat() {
return "^" + regex.getRegex() + "{" + minimum + "," + maximum + "}";
}

and:

public enum Regex {
    LowercaseAlphabetic("[a-z]"), UppercaseAlphabetic("[A-Z]"), Numerical("[0-9]"), Alphabetic("[a-zA-Z]"),
AlphaNumerical("^[A-Za-z0-9_ ]*$");

    public String regexValue;

    Regex(String regexValue) {
        this.regexValue = regexValue;
    }
}
}

and:

class EntityDao {
public void save(Entity entity) {
    System.out.println("Saving the model!");
}
}

and:

class Entity {

private String name;
private String idCard;
private String address;

public void setIdCard(String idCard) {
    this.idCard = idCard;
}

public void setName(String name) {
    this.name = name;
}

public void setAddress(String address) {
    this.address = address;
}

public String getIdCard() {
    return idCard;
}

public String getIdName() {
    return name;
}

public String getAddress() {
    return address;
}
}
StepUp
  • 36,391
  • 15
  • 88
  • 148
JamesB
  • 569
  • 1
  • 9
  • 31
  • I see that you've reposted an edited and improved version of [your previous deleted question](https://stackoverflow.com/q/73839241/), one that now shows an attempt to post a [mre] in the question, and this is a good thing, but I will ask for a few more things: 1) please respond to comments to your questions -- you left my previous comments somewhat high and dry, not knowing if any made sense or not. 2) please [edit] this post in an effort to make the post compilable and runnable, which it currently isn't. It refers to a Model & ModelDao class that we don't have and has duplicate constructors. – Hovercraft Full Of Eels Sep 24 '22 at 23:37
  • Try [Hibernate Validator](https://hibernate.org/validator/documentation/getting-started/). – Paul Samsotha Sep 24 '22 at 23:44
  • @HovercraftFullOfEels Okay, all fixed now, I'm sorry that I didn't answer the last comment, I didn't understand the concept of predicates with HashMap by the way or making an isolated method to it all. – JamesB Sep 25 '22 at 00:33
  • @PaulSamsotha Trying to make it without using any framework. – JamesB Sep 25 '22 at 00:34

1 Answers1

0

We have a repetetive behaviour:

if (saveToDatabase) {
    ValidationEntity nameValidation = new 
        ValidationEntity(ValidationEntity.Regex.Alphabetic, 
            name, "ID Card", 0, 13);
    saveToDatabase = nameValidation.isValid();
    entity.setName(name);
    // create ValidationEntity object
    // check whether entity is valid 
    // set some attribute value
}

I would suggest:

  • to collect all validation logic in one place
  • set some attribute values after validation is completed
  • and then save to database

By doing the above actions, we will separate our validation logic, variable assignments and saving database. So our code should comply with Single Responsibility principle of SOLID principles.

So let's put repetetive behaviour in some abstraction. I am sorry, I am not Java guy. Let me show via C#. But I've provided comments of how code could look in Java:

public abstract class FieldValidation
{
    // I am not Java guy, but if I am not mistaken, in Java,
    // if you do not want method to be overriden, you shoud use `final` keyword
    public abstract bool IsValid();
}

And its concrete implementations:

public class IdCardFieldValidation : FieldValidation // extends in Java
{
    public override bool IsValid() // @Override in Java
    {
        // your code here:
        /*ValidationEntity nameValidation = new
            ValidationEntity(ValidationEntity.Regex.Alphabetic,
                name, "ID Card", 0, 13);
        return nameValidation.isValid();*/
        return true;
    }
}

public class EmailFieldValidation : FieldValidation // extends in Java
{
    public override bool IsValid() // @Override in Java
    {
        // your code here:
        /*ValidationEntity emailValidator = new 
            ValidationEntity(ValidationEntity.Regex.AlphaNumerical, 
                address, "Address", 1, 9);
        returnr emailValidator.isValid();*/
        return true;
    }
}

And then we can create a class which will have all validations. And this class will return whether all validations were okay:

public class AllFieldValidations
{
    private List<FieldValidation> _fieldValidations = new List<FieldValidation>()
    {
        new IdCardFieldValidation(),
        new EmailFieldValidation()
    };

    public bool IsValid() 
    {
        foreach (var fieldValidation in _fieldValidations)
        { 
            if (!fieldValidation.IsValid())
                return false;
        }

        return true;
    }
}   

And then your method will look like this:

AllFieldValidations allFieldValidations = new AllFieldValidations();
bool isAllFieldsValid = allFieldValidations.IsValid();

if (!isAllFieldsValid)
    return;

SetAttributes();
SaveToDatabase();

and implementations of other methods:

public void SetAttributes
{
    entity.setName(name);      
    entity.setIdCard(idCard);  
    entity.setAddress(address);

}

public void SaveToDatabase()
{
    new EntityDao().save(entity);
}   

So here we've applied Single responsibility principle here of SOLID principles.

I mean we do not have almighty method that has all validations, setting attributes and saving to database.

Pros of this approach:

  • highly testable code
  • great separation of concerns
  • improved readability. We've created methods with self explanatory names
StepUp
  • 36,391
  • 15
  • 88
  • 148