3

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
  • 2
    Even in Java 8... this will still look ugly Commented Aug 17, 2018 at 9:28

7 Answers 7

3

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])));
Sign up to request clarification or add additional context in comments.

7 Comments

This is just another way of if checks. Though you squeezed it.
@Raghuveer: It avoids the bloater, null checks, is more readable and null-safe. Moreover, this is a correct way to work with Optional.
I have edited my answer adding a shortcut using Map.
@Nikolas a well deserved +1 for the last variant, I was working on something close to that, but you already posted
Because there are duplicate conditions, it's best to define it firstly and then put it on the map as value.
|
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.

7 Comments

I deleted my Lombok comment since OP does not seem to want a null-check enforcing non-null values but rather a way to do different things depending on the null values.
@luk2302 It might be helpful, so I kept it in. But I also updated my answer to focus on the better approach (bean validation), too. Basically boiling down to what the other answer suggests, too ;-)
@GhostCat well this is not about validation per-se looks like, as such I don't think that this answers the question (at least two people disagree though)
@GhostCat we have a joke here in my country about this, about a student who studied veterinary and on a certain exam all he studied was about "hair". So he gets a question about frogs and his answer is "well if a frog had hair, this is what might have happened... ". I truly hope you will not get offended by this, it's just my view on the matter ;)
But also... if OP is looking the wrong way, shouldn't we at least make an effort to point them in the right direction?
|
2

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.

3 Comments

Doesn't solve the problem because it just adds a constraint to the members.
@nguyentt I have to agree, no idea how this solves OP's problem
His problem is to get ride of if checks only though he has given other unrelated code.
1

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 Comments

I was going for that but using a Map<Predicate, String> to also store the error message. Just need to stream the map, filtrer the result and append the StringBuilder with the value.
@AxelH A Map would be my first idea too. The MANDATORY_EXCESS_FIELDS array is cumbersome. In fact I started the answer before the question received code; and I think a boolean[] array would suffice; much nicer one cannot get.
@AxelH if only Predicate would override equals/hashCode that is
@Eugene good thing I didn't tried ;-) but this can be fixed using a Pojo "Error" with an ID and the message. This could be hashed. Thanks for this details.
@AxelH or a class that implements Predicate, of course
1

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(""));

Comments

0

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()]);
    }
}

1 Comment

run this in parallel and see it failing. map should be side effect free, and it's not in your case
0

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);
    }
 }

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.