# Refactoring multiple boolean conditions?

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
}


filterBasedOnConsist* are five-line methods. Above several AND conditions doesn't look clean. if there a way to improve this logic?

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

• @mtj non taken ;) – pepijno Mar 24 at 14:09
• @GovindaSakhare business requirements should not be dictating your implementation choice. What is the REST service (value-)adding into the String representation? Perhaps you should even consider deserializing that representation to some kind of EnrichedEmployee class and perform the filtering on that data object. String-based filtering is brittle and can be easily undone with data quality issues such as a stray delimiter. – h.j.k. Mar 26 at 3:23

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.

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.

• This seems like a fairly inefficient way to reimplement the Predicate.and(...) method. – TorbenPutkonen Mar 24 at 14:51
• @TorbenPutkonen you are probably right, I forgot about the Predicate.and(...) method – pepijno Mar 24 at 15:52