2
\$\begingroup\$

I am currently reworking our entity code generator, that uses JaxB for schema validation, and Stringtemplate for code generation.

As we are giving our professional service the possibility to add Validators to the attributes of the entities they create, I need to wrap each parsed Field, in order to add the Validators.

The following code works, but I do not feel comfortable with it:

@SuppressWarnings( "boxing" )
private Map<String, Object> wrapField( Field f )
  {
    HashMap<String, Object> result = new HashMap<>();
    if ( f.isGenerateGetter() )
     {
      DecimalMaxValidator decMax = null;
      DecimalMinValidator decMin = null;
      NotBlankValidator notBlank = null;
      NotNullValidator notNull = null;
      LengthValidator lengthVal = null;

      result.put( "generateGetters", f.isGenerateGetter() );
      result.put( "getterAddOverride", f.isGetterAddOverride() );
      for ( Validator v : f.getValidators().getValidators() )
      {
        HashMap<String, Object> tmp = new HashMap<>();
        if ( v instanceof DecimalMaxValidator )
        {
          decMax = addDecimalMaxValidator( result, decMax, v, tmp );
        }
        else if ( v instanceof DecimalMinValidator )
        {
          decMin = addDecimalMinValidator( result, decMin, v, tmp );
        }
        else if ( v instanceof NotBlankValidator )
        {
          notBlank = addNotBlankValidator( notBlank, v, tmp );
        }
        else if ( v instanceof NotNullValidator )
        {
          notNull = addNotNullValidator( notNull, v, tmp );
        }
        else if ( v instanceof LengthValidator )
        {
          lengthVal = addLengthValidator( lengthVal, v, tmp );
        }
        else
        {
          // TODO: error: Wrong type? type not found?
        }

      }
    }
    return result;
  }

  private NotNullValidator addNotNullValidator( NotNullValidator notNull, Validator v, HashMap<String, Object> tmp )
  {
    if ( notNull == null )
    {
      notNull = (NotNullValidator) v;
      tmp.put( "notNullValidator", true );
      tmp.put( "notBlankValidatorTransKey", v.getTranslationKey() );
    }
    else
    {
      // TODO error: double dec
    }
    return notNull;
  }

  @SuppressWarnings( "boxing" )
  private NotBlankValidator addNotBlankValidator( NotBlankValidator notBlank, Validator v, HashMap<String, Object> tmp )
  {
    if ( notBlank == null )
    {
      notBlank = (NotBlankValidator) v;
      tmp.put( "notBlankValidator", true );
      tmp.put( "notBlankValidatorTransKey", v.getTranslationKey() );
    }
    else
    {
      // TODO error, double dec
    }
    return notBlank;
  }

  @SuppressWarnings( "boxing" )
  private DecimalMinValidator addDecimalMinValidator( HashMap<String, Object> result, DecimalMinValidator decMin, Validator v,
      HashMap<String, Object> tmp )
  {
    if ( decMin == null )
    {
      decMin = (DecimalMinValidator) v;
      tmp.put( "value", decMin.getValue() );
      tmp.put( "inclusive", decMin.isInclusive() );
      tmp.put( "translationKey", decMin.getTranslationKey() );
      result.put( "decimalMinValidator", tmp );
    }
    else
    {
      // TODO error, double dec
    }
    return decMin;
  }

  @SuppressWarnings( "boxing" )
  private DecimalMaxValidator addDecimalMaxValidator( HashMap<String, Object> result, DecimalMaxValidator decMax, Validator v,
      HashMap<String, Object> tmp )
  {
    if ( decMax == null )
    {
      decMax = (DecimalMaxValidator) v;
      tmp.put( "value", decMax.getValue() );
      tmp.put( "inclusive", decMax.isInclusive() );
      tmp.put( "translationKey", decMax.getTranslationKey() );
      result.put( "decimalMaxValidator", tmp );
    }
    else
    {
      // TODO error, double dec
    }
    return decMax;
  }

  private LengthValidator addLengthValidator( LengthValidator lengthVal, Validator v, HashMap<String, Object> tmp )
  {
    if ( lengthVal == null )
    {
      lengthVal = (LengthValidator) v;
      tmp.put( "min", lengthVal.getMin() );
      tmp.put( "max", lengthVal.getMax() );
      tmp.put( "translationKey", lengthVal.getTranslationKey() );
    }
    else
    {
      // TODO error, double dec
    }
    return lengthVal;
  }

There are no explanations yet, but the code should be mostly self-explanatory.

Comparing the outsourced addXXXValidator methods, feels awkward regarding redundant code.

As I am not very familiar with the java generics, I need your help with refatoring this code, so it gets smaller, faster and better maintainable.

How can I make the addXXXValidator methods more "beautiful"?

Note:

I am not allowed to edit the Validator classes.


Explanation to the getValidators()call:

f.getValidators()  // Wrapper, returns a Validators object
 .getValidators()  // "second call", returns a List of Validator objects

while there is the Validators object: (the types are our internal types)

<complexType name="validatorsType">
   <complexContent>
     <restriction base="{http://www.w3.org/2001/XMLSchema}anyType">
       <choice maxOccurs="unbounded" minOccurs="0">
         <element name="notNullValidator" type="notNullValidatorType"/>
         <element name="notBlankValidator" type="notBlankValidatorType"/>
         <element name="decimalMaxValidator" type="decimalMaxValidatorType"/>
         <element name="decimalMinValidator" type="decimalMinValidatorType"/>
         <element name="lengthValidator" type="lengthValidatorType"/>
         <element name="patternValidator" type="patternValidatorType"/>
       </choice>
     </restriction>
   </complexContent>
 </complexType>

and the Validator class:

 <complexType name="validatorType">
   <complexContent>
     <restriction base="{http://www.w3.org/2001/XMLSchema}anyType">
       <attribute name="translationKey" type="{http://www.w3.org/2001/XMLSchema}string" />
     </restriction>
   </complexContent>
 </complexType>

The NotNullValidator :

 <complexType name="notNullValidatorType">
   <complexContent>
     <extension base="validatorType">
     </extension>
   </complexContent>
 </complexType>

The DecimalMaxValidator :

 <complexType name="decimalMaxValidatorType">
   <complexContent>
     <extension base="validatorType">
       <attribute name="value" use="required" type="{http://www.w3.org/2001/XMLSchema}string" />
       <attribute name="inclusive" type="boolean" default="true" />
     </extension>
   </complexContent>
 </complexType>

The DecimalMinValidator is similar to DecimalMaxV. The NotBlankV similar to NotNullV.

The LengthValidator:

 <complexType name="lengthValidatorType">
   <complexContent>
     <extension base="validatorType">
       <attribute name="min" type="{http://www.w3.org/2001/XMLSchema}integer" default="0" />
       <attribute name="max" type="{http://www.w3.org/2001/XMLSchema}integer" default="2147483647" />
     </extension>
   </complexContent>
 </complexType>

While the above Schemafragments represent the

Expected content of the class (taken from the class documentation)

\$\endgroup\$
  • \$\begingroup\$ I assume you mean decMax = addDecimalMaxValidator( result, decMax, v, result); in each wrapField else if? \$\endgroup\$ – MrBrushy Jan 18 '17 at 15:12
  • \$\begingroup\$ What does the field.getValidators() return? It could be interesting. If its a custom validators holder object, it might hold the key to the solution. There is code lacking, would you mind sharing it? I don't like guessing too much. \$\endgroup\$ – MrBrushy Jan 18 '17 at 15:23
  • \$\begingroup\$ @SylvainBoisse No, I am setting the variables decMax, decMin, notBlank etc.to the return values of the outsourced functions. I will update the question in order to share the meaning of getValidators() \$\endgroup\$ – Do Re Jan 18 '17 at 15:38
  • \$\begingroup\$ @DoRe You cannot change the Validator class. Can you change the Validators class? I suspect you can't either. \$\endgroup\$ – MrBrushy Jan 18 '17 at 16:09
  • \$\begingroup\$ @SylvainBoisse No, I cannot change any class besides the one I am currently writing. Anyway, I like the idea mtj had in his answer. I will update my code and question accordingly \$\endgroup\$ – Do Re Jan 19 '17 at 7:20
3
\$\begingroup\$

You could replace your instance-check with a configuration map, which holds the class as the key and a consumer as the values.

Some improvised code to illustrate the principle:

public class Try43 {
    // mockup of your classes and hierarchy
    private static interface Validator {}
    private static class TestValidator implements Validator {}

    public static void main(String[] args) {
        // Build and fill the map at some central point:
        Map<Class<? extends Validator>, Consumer<Validator>> config = new HashMap<>();
        config.put(TestValidator.class, v -> doSomething("constantparam", v));

        // This replaces your if-else-cascade:
        Validator v = new TestValidator();
        if(config.containsKey(v.getClass()))
            config.get(v.getClass()).accept(v);
        // else error-handling
    }

    private static void doSomething(String someParameter, Validator val) {
    }
}

... if you need to pass a local tmp-map, this will be more BiConsumer-ish in your case, but the idea should be clear.

\$\endgroup\$
  • \$\begingroup\$ I like this idea! I will update my code and then update my answer. Maybe there is something to be done about the functions as well. Thank you so far! \$\endgroup\$ – Do Re Jan 19 '17 at 7:19
  • \$\begingroup\$ after refactoring my code according to your solution, is looks so much better, that I decided to not split the functions anymore, but use enum values as constantParam(as you called it) and simple switch case over it \$\endgroup\$ – Do Re Jan 19 '17 at 11:54

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.