0
\$\begingroup\$

I have a method where I fetch user input, check if certain values exist, and based on that build my own custom input object that I would use to search in a database. The code for the search method is as follows.

public SearchDocumentResult searchData(EmployeeInput employeeInput) {
    EmployeeInput getEmployeeInputForSearch = buildApplicationInputForSearch(employeeInput);
    if (employeeInput != null) {
        return super.searchForExactData(getEmployeeInputForSearch);
    } else {
        return super.searchForCloseMatchData(getTestPersonInput);
    }
}

The methods with multiple if checks on the input are as follows. Both the below methods and the above method exist in the same class.

private Application buildApplicationInputForSearch(Application applicationInput) {
    Application.Builder applicationForSearch = Application.builder();
    String jobIdFromInput = applicationInput.getJobId();
    applicationForSearch.withIsTagged(applicationInput.isTagged());
    if (!StringUtils.isEmpty(jobIdFromInput)) {
        applicationForSearch.withJobId(jobIdFromInput);
    }
    FormSection formSectionInput = applicationInput.getFormSection();
    if (formSectionInput != null) {
        this.buildFormSectionInputForSearch(formSectionInput);
    }
    return applicationForSearch.build();
}

private FormSection buildFormSectionInputForSearch(FormSection formSectionInput) {
    FormSection.Builder formSectionForSearch = FormSection.builder();
    String formCountry = formSectionInput.getCountry();
    Map<String, String> employeeQuestions = formSectionInput.getEmployeeQuestions();
    Map<String, String> applicationQuestions = formSectionInput.getApplicationQuestions();
    List<String> formNames = formSectionInput.getNames();
    if (!StringUtils.isEmpty(formCountry)) {
        formSectionForSearch.withCountry(formCountry);
    }
    if (formNames.size() > 0) {
        formSectionForSearch.withNames(formNames);
    }
    if (employeeQuestions.size() > 0) {
        formSectionForSearch.withEmployeeQuestions(employeeQuestions);
    }
    if (applicationQuestions.size() > 0) {
        formSectionForSearch.withApplicationQuestions(applicationQuestions);
    }
    return formSectionForSearch.build();
}

The EmployeeInput is a model class that gets generated through a library and therefore I cannot make that use Java Optional for fields that may or may not exist. Using this EmployeeInput object as it is, how can I make this code more readable, with less if conditions? Any help would be much appreciated.

\$\endgroup\$
1
\$\begingroup\$
if (formSectionInput != null) {
        this.buildFormSectionInputForSearch(formSectionInput);
    }

can be replaced with:

Optional.ofNullable(formSectioninput).map(this::buildFormSectionInputForSearch);

I dont understand the need of if condition in the statements like below:

if (formNames.size() > 0) {
        formSectionForSearch.withNames(formNames);
    }

If you are expecting that formNames can be null then you should not use .size(), it can throw NullPointer exception. You should do similar to above example:

Optional.ofNullable(formNames).map(formSectionForSearch::withNames);
\$\endgroup\$
0
\$\begingroup\$

Move the condition - if it is not contextual - into the guarded method:

if (!StringUtils.isEmpty(formCountry)) {
    formSectionForSearch.withCountry(formCountry);
}
if (formNames.size() > 0) {
    ...
}

So:

formSectionForSearch.withCountry(formCountry);
formSectionForSearch.withNames(formNames);

void withCountry(String formCountry) {
    if (!StringUtils.isEmpty(formCountry)) {
        ...
    }
}

void withNames(List<String> formNames) {
    if (!formNames.isEmpty()) {
        formSectionForSearch.withNames(formNames);
    }
}

On reuse in N call sites (here unlikely) it will even reduce code. And it creates more secure code for calling the method.

\$\endgroup\$

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.