Optimizing methods with multiple if checks on getter values

Question

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.

Does this answer your question? [Style for control flow with validation checks](https://softwareengineering.stackexchange.com/questions/148849/style-for-control-flow-with-validation-checks) — gnat, Jun 08 '21 at 16:29
@gnat thank you for the suggestion. I'm struggling to understand how I could apply specification pattern as there is a Builder involved here. Sounds confusing to think about creating individual methods that would return part of the builder for each validation. Would appreciate if you could help with an answer applying that :) — AnOldSoul, Jun 08 '21 at 16:34
It's unclear why you need to make those checks. Can't the builder see when a parameter is effectively empty and therefore can be ignored? — Shadows In Rain, Jun 09 '21 at 11:40
@ShadowsInRain Oh yes it can, but the checks here are to make sure that I don't pass some of the existing values of the builder to the search method. Therefore I check if the "searchable" values exist and create an object including only them (eliminating other existing values as well) — AnOldSoul, Jun 09 '21 at 13:36

Blaž Mrak · Answer 1 · 2021-06-13T00:20:14.003

You can have the first part as follows:

private Application buildApplicationInputForSearch(Application applicationInput) {
        return Application.builder()
                .withJobId(applicationInput.getJobId())
                .withIsTagged(applicationInput.isTagged())
                .withFormSection(applicationInput.getFormSection())
                .build();
    }

then in the Application.Builder you have the following

// the simple case
public Builder withJobId(String jobId) {
  if(isNotEmpty(jobId)) {
    this.jobId = jobId;
  }

  return this;
}

...
// for the complex case create another builder
public Builder withFormSection(FormSection formSection) {
  this.formSectionInput = FormSectionInput.builder()
      .withWhatever(formSection.getWhatever())
      ...
      ...
      .build();
}

Move all the if statements that you have around the builder calls, into the builder methods. This also makes your buildFormSection method unnecesary. The "optional" logic will now be in the builder and you will not have to worry about it when creating the query input. If you want to allow empty strings or arrays, you can overload the builder method with another boolean flag.

So in addition to the standard withSomething(String something) you also have:

public Builder withSomething(String something, boolean skipValidation) {
  if(skipValidation || isNotEmpty(something)) {
    this.something = something;
  }

  return this;
}

Edit: if you want to convey that there is some validation within the builder methods, you can call them withValidSomething, which removes the need for the withSomething(..., boolean skipValidation).

Optimizing methods with multiple if checks on getter values

1 Answers1