In a recent PR, a developer, whom I will call Alice, came across a lot of resistance by a coworker (Bob) because she wrote a utility code unit in a fluent style rather than in a classical style.
In short, Alice had a piece of business logic (BL) similar to this (Java):
Role role = ...;
List<Role> otherRoles = ...;
if ( /* role is higher rank than all other roles */ ) {
// do stuff
}
where the enumeration Role
must have a ranking system according to some business logic and she needed a utility method to compare a single role against each one of a set of other roles.
So a classical approach would be like this:
if (isRoleHigherThanAll(role, otherRoles)) {
// do stuff
}
And you would have a classic Utility method like this:
public static boolean isRoleHigherThanAll(Role role, Collection<Role> otherRoles) {
// utility method logic here...
}
However, since those lines of code were part of some complex business logic method, Alice wanted to make it "fluent" to improve the readability and make it clear what the condition is checking at a first glance for those who are not familiar with such code.
if (role(role).isHigherRankThanAll(otherRoles)) {
// do stuff
}
And the Utility methods would look like this instead:
static ComparableRole role(Role role) {
return new ComparableRole(role);
}
// intermediate object serving the fluent-style API
@Data
static class ComparableRole {
private final Role role;
public boolean isHigherRankThanAll(Collection<Role> otherRoles) {
// utility method logic here
}
}
As you can see, the utility logic LOC are sort of doubled, but in return you get a fluent, allegedly more readable condition in the main business logic method.
Now, I have just massively simplified what Alice actually did... The condition was a bit more complex and Alice went much further and added another couple of utility methods in the fluent interface style to create her own domain-specific language (DSL) for this single isolated problem.
The main point is that she received objections in the PR that such style makes a verbose Utility API which is pointless given that such Utility API only exists to break a complex BL class and externalize a particular aspect of the logic into a different unit of code, so as a matter of fact it is only used in one single place. Bob says that the alternative doesn't read that bad (it actually reads more natural) and since nobody would write code like that, such coding style breaks the POLA. Also, with the classic way, the utility code LOC are more than halved.
Alice thinks it is just a stylistic debate and at the end of the day her solution is good enough, but Bob went as far as rejecting the PR as Alice's approach was deemed actually "wrong" and not up to standards.
I thought I'd ask for some external opinions. I am using fictitious names to prevent biases and focus on the matter.
Is there "too far" with a fluent interface and method chaining? And how far is too far?
isRoleHigherThanAll
is oddly specific. I guess there is a utility set of many such specific use case functions which keeps growing ?