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?
6
When it comes to readability, the opinion of the one who wrote the code should never be trusted over someone who didn’t. It’s called the curse of knowledge.= If you wrote it, you already know how it works. So you have no idea how easy it is to read. So if Bob is that opposed, Allice’s only hope is what Charlee thinks.
Allice’s real mistake was not shopping the new idea around early and getting buy-in before writing a bunch of code that dies in a big formal peer review. New ideas need time. They need to be shared one on one and hashed out. A process that would shape how Allice applies the new idea and would familiarize her team with the new style.
As for the fluent style itself, I’ve heard you say some concerning things. I have some experience with the fluent style= myself. One thing I learned is setting up an internal DSL is a fair bit of work. So when you say it’s only going to be used in one place that left me very concerned.
The ideal situation for a big DSL is when it will be used often. When its attention to a specialized situation can be leveraged, as it’s used over and over. That’s when the pain of designing and learning this beastie is really worth it.
Yes, it is a more natural style. But until the team is used to it it’s an unfamiliar style. Getting them familiar with it comes at a cost. I know it’s fun to play with new toys, but you won’t win any friends by throwing your toys at them. Share them gently and you might find they have a few toys worth playing with themselves.
Also, understand that these things come in very different sizes. You’ll see a fluent style can be monstrously complex= or more moderate like Java 8 streams= or as simple as a Joshua Bloch Builder.= The simpler they are the less wide spread their use needs to be to make them worth it.
P.S.: I will say though, better names help a fluent interface a bunch:
if ( role(admin).outranks(annoyingUsers) ) { ... }
P.P.S.: Another answer is challenging my central thesis: writers can’t be trusted to know what’s readable. They need to listen to feedback from readers. So I’ll add some links that discuss a closely related issue:
Code is read far more often then it is written.===
P.P.P.S: Seems I need to clarify how the voting should work. It’s not that Bob (or any single reader) should have veto power. It’s that Alice (our writer) should have no vote. She’s biased. So her only hope is that my creatively spelled Charlee (fellow reader) votes for the code producing a tie which we can resolve with a coin flip. Arbitrary, but it gets us out of the conference room in time for lunch.
8
I agree with Bob, fluent does nothing to improve code and adds an extra class/method in most cases.
However.
This is why you have rules for PRs and don’t just leave it down to preferences. If there isn’t a rule which says “you must not use the fluent style” then the PR shouldn’t be rejected.
Given that the code works either way, it is just a coding style issue and developers should be able to cope with multiple different coding styles without any problems. If Bob can’t, he needs to learn.
If you don’t write down your rules first, then you will just make everyone’s lives harder as their PRs get rejected for made-up crap.
6
It was this remark by @candied_orange that prompted me to add this answer:
the opinion of the one who wrote the code should never be trusted over someone who didn’t
I would adopt the opposite approach by default, that the decisions of the writer(s) should be accorded respect.
It is the writer who has to do the bulk of the work and get to grips with the problem in the greatest detail, and allowing any one reader to override them is far too capricious.
The criteria for override should be that “the results are so bad that no reasonable programmer would have produced them in the circumstances”.
In other words, you are orienting the question not about whether the code is as good as it possibly can be (which is bound to create bitter arguments on minor points), but whether it is bad beyond the standard of competence.
I’m not myself a fan of the “fluent” style (perhaps because I’ve always been a user of VB and it has a With block that solves the same problem), but I haven’t heard that the general level of regard for the fluent style is so low that no reasonable programmer would ever use it.
When conformity is required on aesthetic matters, the best approach would be to either involve more people in decisions earlier, or to give design responsibility to a single person, rather than to delegate but quibble with the results afterwards.
8
From what I see, the pure functionality should have been implemented as a method in the Role class, named outranks taking another role.
So everybody in the future can check whether one role outranks another without jumping through silly hoops like making a list out of the one element they have first to then call a function that only works on lists.
From there, it really just becomes a matter of how well your language can solve simple problems.
Role thisRole = ?;
if(otherRoles.All(otherRole => thisRole.outranks(otherRole)))
or even shorter:
if(otherRoles.All(thisRole.outranks))
this is C#, but I’m sure the Java equivalent looks about the same.
Once you have your basic methods straightened out, there is little need for this “utility” function.
You said your example is more complicated, but my advice would be the same: boil it down to the basics and implement those basics that are missing. Then use your programming language to solve problems, I’m sure it can. That’s way better than writing “utility” and mixing your basic domain concepts with your logic at the site you want to use them.
1