Refactor multiple If' statements in Java-8

问题:

I need to validate mandatory fields in my class

For example, 9 fields must not be null.

I need to check if they are all null but I am using multiple if statements for this now as below:

StringBuilder mandatoryExcessFields = new StringBuilder(MANDATORY_EXCESS_FIELDS.length);

if(Objects.isNull(excess.getAsOfDate())){
    mandatoryExcessFields.append(MANDATORY_EXCESS_FIELDS[0]);
}

if(StringUtils.isEmpty(excess.getStatus())) {
    mandatoryExcessFields.append(MANDATORY_EXCESS_FIELDS[1]);
}

if(Objects.isNull(excess.getLimit())) {
    mandatoryExcessFields.append(MANDATORY_EXCESS_FIELDS[2]);
}

if(!Objects.isNull(excess.getLimit()) && Objects.isNull(excess.getLimit().getId())) {
    mandatoryExcessFields.append(MANDATORY_EXCESS_FIELDS[3]);
}

if(!Objects.isNull(excess.getLimit()) && Objects.isNull(excess.getLimit().getAsOfDate())) {
    mandatoryExcessFields.append(MANDATORY_EXCESS_FIELDS[4]);
}

if(Objects.isNull(excess.getExposure())) {
    mandatoryExcessFields.append(MANDATORY_EXCESS_FIELDS[5]);
}

if(!Objects.isNull(excess.getExposure()) && Objects.isNull(excess.getExposure().getCoordinates())) {
    mandatoryExcessFields.append(MANDATORY_EXCESS_FIELDS[6]);
}

if(!Objects.isNull(excess.getExposure()) && Objects.isNull(excess.getExposure().getValue())) {
    mandatoryExcessFields.append(MANDATORY_EXCESS_FIELDS[7]);
}

if(StringUtils.isEmpty(excess.getLimitValue())) {
    mandatoryExcessFields.append(MANDATORY_EXCESS_FIELDS[8]);
}

Do we have a better approach to reduce this boilerplate code or any design pattern or any new feature from Java-8 which I can leverage?

回答1:

All the Object.isNull might be replaced with Optional object and its methods. Let's take example the line:

if (!Objects.isNull(excess.getLimit()) && Objects.isNull(excess.getLimit().getId())) {
    mandatoryExcessFields.append(MANDATORY_EXCESS_FIELDS[3]);
}

Would be simplified to (and squeezed on 1 line remains readable):

Optional.ofNullable(excess.getLimit())                                // check the Limit
        .map(limit -> limit.getId())                                  // if not null, getId
        .ifPresent(i -> builder.append(MANDATORY_EXCESS_FIELDS[3]));  // Append if present

And for the String.isEmpty(s) check, you have to create Optional in this way:

Optional.ofNullable(excess.getStatus()).filter(s -> !StringUtils.isEmpty(s))

---

A short way would be to pass those Optional object into the map and use the index to iterate through them and perform an action. int count is a number of checkings:

Map<Integer, Optional<?>> map = new HashMap<>();
map.put(...);
map.put(1, Optional.ofNullable(excess.getStatus()).filter(s -> !StringUtils.isEmpty(s)));
map.put(...);
map.put(3, Optional.ofNullable(excess.getLimit()).map(limit -> limit.getId()));
map.put(...);

for (int index=0; index<count; index++) {
    map.get(index).ifPresent(any -> mandatoryExcessFields.append(MANDATORY_EXCESS_FIELDS[index]));
}

And the for-cycle might be simplified as well:

IntStream.range(0, count).forEach(index -> 
    map.get(index)
       .ifPresent(any -> mandatoryExcessFields.append(MANDATORY_EXCESS_FIELDS[index])));

回答2:

Basically, there are two ways here:

• As suggested by the comment, NonNull as offered by Project Lombok for example
• Java bean validation

I would heavily recommend to look into bean validation:

Define your classes that carry information as beans. And then use the wide range of annotations to mark the corresponding fields. And then use some existing framework to do the validation for you. You can even define your own annotations there, that run your own code.

回答3:

You can use javax.validator and hibernate.validator with @NotNull annotation on each field (or whichever field you want) on your excess POJO class. This combination provides an extensive pattern checking as well.

By this you don't have to do all the if checks explicitly. You can get ride of not only null checks but also pattern matching checks which can get scattered all over your code.

回答4:

Basically the initialisation and assignments should not set any field to null.

If this is unopportune (a field being really logically optional), the field should probably be an Optional<...>, assigned with an Optional.ofNullable(...). This ensures that at usage the field is safely processed, but causes editing work of course.

Seeing the code now, here it seems that there is no easy refactoring.

The code could be refactored; somewhere a mapping of features is missing.

Predicate<Excess>[] parts = {
    exc -> Objects.isNull(exc.getAsOfDate()),
    exc -> StringUtils.isEmpty(exc.getStatus()),
    ...
};
for (int i = 0; i < parts.length; ++i) {
    if (parts[i].test(excess)) {
        mandatoryExcessFields.append(MANDATORY_EXCESS_FIELDS[i]);
    }
}

Or such.

回答5:

As easy refactoring you could introduce two helper methods :

private String createErrorMsgIfObjectNull(Object o, String errorMessage) {
    return Objects.isNull(o) ?  errorMessage : "";
}

private String createErrorMsgIfStringEmpty(String s, String errorMessage) {
    return StringUtils.isEmpty(s) ?  errorMessage : "";
}

And use them in this way :

StringBuilder mandatoryExcessFields = new StringBuilder(MANDATORY_EXCESS_FIELDS.length);

mandatoryExcessFields.append(createErrorMsgIfObjectNull(excess.getAsOfDate(), MANDATORY_EXCESS_FIELDS[0]))
                     .append(createErrorMsgIfStringEmpty(excess.getStatus(), MANDATORY_EXCESS_FIELDS[1]))
                     .append(createErrorMsgIfObjectNull(excess.getLimit(), MANDATORY_EXCESS_FIELDS[2]))
                     // ...  

By checking the type of the object to test you could still go further. You would have a single helper method that will apply the processing according to the argument type :

private String createErrorMsgIfNullOrEmptyString(Object o, String errorMessage) {
    if (o instanceof String) {
        return StringUtils.isEmpty((String)o) ? errorMessage : "";
    }
    return Objects.isNull(o) ? errorMessage : "";
}

---

A Java 8 stream way would inline the helper in a filter and map() operations and would collect the String result :

List<SimpleImmutableEntry<Object, String>> objectAndErrorMessageList = new ArrayList<>();
int i = 0;
objectAndErrorMessageList.add(new SimpleImmutableEntry<>(excess.getAsOfDate(), MANDATORY_EXCESS_FIELDS[i++]));
objectAndErrorMessageList.add(new SimpleImmutableEntry<>(excess.getStatus(), MANDATORY_EXCESS_FIELDS[i++]));
// and so for

String globalErrorMsg = 
    objectAndErrorMessageList.stream()
                             .filter(e -> {
                                 Object objectToValid = e.getKey();
                                 if (objectToValid == null) {
                                     return true;
                                 }
                                 if (objectToValid instanceof String && StringUtils.isEmpty(objectToValid)) {
                                     return true;
                                 }
                                 return false;
                             })
                             .map(SimpleImmutableEntry::getValue)
                             .collect(Collectors.joining(""));

回答6:

Other solution would be like this: same as @Nikolas answer.

Map<Integer, Predicate<Excess>> map = new HashMap<>();

Predicate<Excess> checkStatus = excess -> excess.getStatus().isEmpty();
Predicate<Excess> checkLimit = excess -> Objects.isNull(excess.getLimit());
Predicate<Excess> checkLimitId = excess -> Objects.isNull(excess.getLimit().getId());
Predicate<Excess> checkLimitAndId = checkLimit.and(checkLimitId);
// other predicates 

map.put(1,checkStatus);
map.put(2,checkLimit);
map.put(3,checkLimitAndId);
// put other predicates ...


for (Map.Entry<Integer, Predicate<Excess>> m : map.entrySet()) {
    if (m.getValue().test(excess)) {
        mandatoryExcessFields.append(MANDATORY_EXCESS_FIELDS[m.getKey()]);
    }
}

回答7:

A little bit complicated, but I have a good solution because it's generic and can be used with any objects:

Excess excess = new Excess(new Limit());

Checker<Excess, Excess> checker = new Checker<>(
    identity(),
    List.of(
        new CheckerValue<>("excess date is null", Excess::getAsOfDate),
        new CheckerValue<>("limit is null", Excess::getLimit)
    ),
    List.of(new Checker<>(Excess::getLimit, List.of(new CheckerValue<>("limit id is null", Limit::getId))))
);

System.out.println(checker.validate(excess));

This code will print:

excess date is null
    limit id is null

The first class Checker contains:

• sourceFunction - for getting the object
• values - for checking each field from object obtained from sourceFunction

• children - a list of Checker

  class Checker<S, T> {
  
  Function<S, T> sourceFunction;
  List<CheckerValue<T>> values;
  List<Checker<T, ?>> children = emptyList();
  
  /*All args constructor; 2 args constructor*/
  
  public String validate(S object) {
      T value = sourceFunction.apply(object);
      if(value != null) {
          String valueString = values.stream().map(v -> v.validate(value)).filter(Optional::isPresent).map(Optional::get).collect(joining("\n"));
          valueString += "\n\t";
          valueString += children.stream().map(c -> c.validate(value)).collect(Collectors.joining("\n"));
          return valueString;
      }
      return "";
  }
  }
  

and CheckerValue class:

class CheckerValue<T> {

    String validationString;
    Function<T, Object> fun;

    /*all args constructor*/

    public Optional<String> validate(T object) {
        return fun.apply(object) != null ? Optional.empty() : Optional.of(validationString);
    }
 }