Refactoring multiple boolean conditions?

Question

I have a Predicate which takes employee object.

Predicate<Employee> getPredicate() {
    return emp -> filter(emp);
}

Now the filter method is very complex, it calls four other methods which returns true/false if all the conditions are true, the predicate will return true.

private boolean filter(Employee employee) {
    String employeeJSONString = employeeToString(employee);
    return filterBasedOnConsistAge(employeeJSONString) && 
           filterBasedOnConsistGender(employeeJSONString) &&
           filterBasedOnConsistNationality(employeeJSONString) &&
           filterBasedOnConsistHandicap(employeeJSONString);
}

private String employeeToString(Employee employee) {
    // converts domainObject to a formatted string, it's a business requirement
}

There are five-line methods, that are linked using logical AND. But the problem here is, the chaining is looking clean. is there a way to improve this logic?

Answer 1

What especially jumps at me is, that you take your real data object (the Employee), convert it to a string representation and do your checks on the string.

Why? Can't you check your data object?

Apart from that, I don't see a problem with 5 and-conditions. This is clearer to read than some clever stream-through-predicates-and-reduce code. Clear. Simple. Leave it like that.

What I'd recommend is rethinking your naming:

• getPredicate(): yes, it returns a predicate, we see that from the method signature. But what does this predicate test?
• filter(): does some filtering, but according to which criteria?

(Sorry to pepijno, I typed this without seeing your answer... no offense meant ;-))

Answer 2

Each one of the filterBasedOnConsist* methods look like they would be individual predicates themselves. So convert each method into a Predicate class and use the default and method to chain them together into a composite predicate:

Predicate<String> employeePredicate =
    new FilterBasedOnConsistAge()
    .and(new FilterBasedOnConsistGender())
    .and(new FilterBasedOnConsistNationality())
    .and(new FilterBasedOnConsistHandicap())

Use a more descriptive name than employeePredicate. I have no idea what you are using it for so I just put a bad generic name there.

Answer 3

One option would be to do it with Streams. This is assuming that all different filters are in the same class as the filter method:

private boolean filter(Employee employee) {
  Stream<Predicate<String>> filters = Stream.of(
    this::filterBasedOnConsistAge,
    this::filterBasedOnConsistGender,
    this::filterBasedOnConsistNationality,
    this::filterBasedOnConsistHandicap
  );

  String employeeJSONString = employeeToString(employee);
  return filters.allMatch(f -> f.test(employeeJSONString));
}

The allMatch method of Stream returns true if the condition is true for all elements in the Stream and false otherwise.