How do I reduce number of FieldValidator derivations?

Question

I am trying to write RSQL Parser which checks if the RSQL is logically correct. while the RSQL Java library checks whether the RSQL expression is grammatically correct, it doesn't check if the expression is logically correct.

One example of logically incorrect expressions would employee.name > 10 as the name is of String type, users should not be able to use >, <, >=, <=

Now to write this logic I am using a factory pattern as below:

public interface FieldValidator {
   public boolean isValidExpression(ComparisonNode comparisonNode);
}

public class NameFieldValidator implements FieldValidator {
   public Set<ComparisonOperator> supportedOperators = Set.of(EQUALS, NOT_EQUALS, IN, NOT_IN);

   public boolean isValidExpression(ComparisonNode comparisonNode) {
       ComparsionOperator operator = comparisonNode.getOperator();
       if(!supportedOperators.contains(operator)) {
          return false;
       }
       // other rules
   }

}

Then I create a factory using FieldValidator implementations and call factory implementation based on the i/p field.

But the problem I am facing here is that I have around 40 fields in my data model and might end up creating 40 implementations here.

Am I over-engineering here? is this not a good use case for a factory pattern? any other design that I should explore?

Answer 1

One should be comparing by type, not by field. One can take a fail-fast approach or try and make a comparison via an under the hood conversion.

Comparing...

Apple > Orange

Doesn't make that much sense.

Apple > "Apple" where "Apple" is a converted Orange.

I convert my orange into an apple. ==> "Name" > "10"

Now I can get an answer.

So, one can check to make sure the same things are being compared and if not throw an exception right away (fail-fast). Or one can make a best effort to always return an answer.

The first way should be less code. The second way needs to take in a lot of scenarios and may return "non-sensical" answers unless specifics are known about the conversion process and language being used.

With this approach, one should have a few classes of comparators. For example, comparators can include ==> String, Number, Boolean, etc. Take the data point on the left and instantiate a comparator by its type and do the comparison. If the right type doesn't match, fail or try convert it to the type of the left. Each comparator can have a list of supported operators based off that type. If the operator isn't supported for that particular type throw an exception.

Answer 2

If you have 40 different rules, each one saying when one of the 40 fields counts as valid, then you need

• 40 different validator functions of the kind isValidExpression
• a mechanism to associate each function with the related field.

Your current solution using a polymorphic interface and 40 different implementations together with a factory is not the worst, but it produces a lot of boilerplate code. Still, modern Java (as well as most other modern versions of major programming languages) provide functional means to design this in a more concise way.

A straightforward solution is to implement the different isValidExpression functions as "standalone objects" of type Function<ComparisonNode,boolean>, put them into a key-value collection like Map<String,Function<ComparisonNode,boolean>> and choose the right validator function for a given field by a simple map lookup (instead of using a factory).

The keys in this map should be a combination of the field name and its class name, so you may be able to implement a generic construction of the key name from the field name using reflection. A more sophisticated approach would be to implement some custom annotations in your data model, so in your data model definition, you can write something along the lines of

class Person
{
     @Validator((cNode) -> Validators.isNameValid(cNode))
     public String Name;
}

I used this static class Validators for holding the different validator functions as an example for the sake of simplicity, I hope you get the idea.

You should also refactor commonalities of different validator functions into a separate, reusable function. From the one example you showed in your question, I cannot tell you what kind of commonalities these are, but I think @JonRaynor has a point: in case you need the same kind of validation for each String field, and in case you have a few of them, refactor that part into a function of it's own.