• Post Reply
  • Bookmark Topic Watch Topic
  • New Topic

How much is too much method chaining?

 
Stephen Jordan
Greenhorn
Posts: 9
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi everybody,

I'm doing a bit of code cleanup/refactoring for my assignment at the moment, focusing on increasing code readability. Using the Stream API I can condense my code down into a couple lines instead of 20, but I'm wondering if all the method chaining has a negative impact on its readability.
For instance, is the following code snippet readable?


Or should I break it out into multiple lines like this?

Any preference for one over the other from a readability respective? I think the first one is a lot more readable but maybe I'm too used to reading my own code to offer an unbiased opinion

Thanks,
Stephen
 
Tim Cooke
Sheriff
Pie
Posts: 3203
142
Clojure IntelliJ IDE Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
When using the Stream library it is designed to be used in that way. Think of it as a sequence or list of operations on a collection, and as such I might format it like a list. For example:
 
Tim Holloway
Saloon Keeper
Posts: 18359
56
Android Eclipse IDE Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I like the second one. It seems to strike a good balance.

I might prefer the first one if your code formatter had been set to keep things more distinct:


Although as a rule, if a chained method has a complex argument, it's a candidate for breakout, both on account of readability and on ease of running a debugger over it. Case in point: I seem to have lost a parenthesis somewhere and a misplaced parenthesis in that particular statement could have radical consequences. If you're lucky, it won't compile. If not

Making a long chain means that you're more insulated against changes in method return types, but as I said, it has its downsides.
 
Tim Cooke
Sheriff
Pie
Posts: 3203
142
Clojure IntelliJ IDE Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
In general the presence of a lot of method chaining is often a warning that you have a code smell called 'feature envy' where you're hoking into the depths of an object outside of your own class to gain access to some feature you wish you had. Stuff like this:

So here the code is really needing to find out that user's allowance for the day, but it has to go digging deeply into the user object for that feature. That's called feature envy, and indicates that the code probably exists in the wrong place.
 
Stephen Jordan
Greenhorn
Posts: 9
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Yes, formatting it correctly does seem to have a large impact on its readability. I left the formatting as the default eclipse setting but I think I'll change it to the suggested splitting for streams.

How do you feel about extracting part of a chain to a method if it's common to several methods?

E.g.
changes to
 
Stephen Jordan
Greenhorn
Posts: 9
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Tim C,

I agree about the method chaining being a code smell when you're just using it to retrieve attributes from an object.
In my code though I'm using it to filter out entries in my cache to retrieve only the information I'm interested in, i.e. 'give me the record number of some record in the cache which has been mark as deleted'.
So I don't think it's guilty of feature entry in this case.

Still I'm a little worried. From a examiner perspective do you think it will affect my grade? Method chaining has traditionally been looked down in the java language and this is a pretty old exam.
I'm definitely going to mention my opinion on how method chaining with the Stream API improves readability in the choices.txt.
 
Tim Cooke
Sheriff
Pie
Posts: 3203
142
Clojure IntelliJ IDE Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
For the stream example, what you have isn't so much method chaining and feature envy, rather a deliberately designed fluent interface.
 
Stephen Jordan
Greenhorn
Posts: 9
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
You're right. Thanks Tim. I think I've just become a bit over-paranoid about my code's readability at this stage. Final stretch time.
 
Roel De Nijs
Sheriff
Posts: 10662
144
AngularJS Chrome Eclipse IDE Hibernate Java jQuery MySQL Database Spring Tomcat Server
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Stephen Jordan wrote:Any preference for one over the other from a readability respective? I think the first one is a lot more readable but maybe I'm too used to reading my own code to offer an unbiased opinion

From the readability perspective, I'm wondering if the old school style (using an enhanced for loop) is not better for this use case...
And a bit related: should you always opt for a stream-based solution when using JDK 8? Or would you still consider an iterator-based solution? And if so, in which situations?
 
Tim Holloway
Saloon Keeper
Posts: 18359
56
Android Eclipse IDE Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I wouldn't recommend anything inherently iterative. These are modern times and often parallel processing is available. But not if you code explicitly for sequential processing.

I routinely give that same advice in the JSF forum. People keep giving me sample code where they've coded a table as a loop in the UI template and I tell them that a table is a 2-dimensional object and best rendered using the element explictly defined to render 2-dimensional objects.
 
Stephen Jordan
Greenhorn
Posts: 9
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Roel De Nijs wrote:I'm wondering if the old school style (using an enhanced for loop) is not better for this use case...


I personally try to avoid instantiating a variable outside a loop and then reassigning it inside it. I just think it complicates the code, especially when nesting is involved.

Roel De Nijs wrote:And a bit related: should you always opt for a stream-based solution when using JDK 8? Or would you still consider an iterator-based solution? And if so, in which situations?


I usually opt for streams in most cases, they outperform iterators in performance and readability in my opinion.
The only time I default to iterators is the rare occasion when I'm working on code which is maintained by developers who haven't moved to JDK 8 and performance isn't an issue. It just leads to a lot of headaches otherwise.
 
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic