amphibient
amphibient

Reputation: 31212

Java performance vs. code-style: Making multiple method calls from the same line of code

I am curious whether packing multiple and/or nested method calls within the same line of code is better for performance and that is why some developers do it, at the cost of making their code less readable.

E.g.

//like
Set<String> jobParamKeySet = jobParams.keySet();
Iterator<String> jobParamItrtr = jobParamKeySet.iterator();

Could be also written as

//dislike    
Iterator<String> jobParamItrtr = jobParams.keySet().iterator();

Personally, I hate the latter because it does multiple evaluations in the same line and is hard for me to read the code. That is why I try to avoid by all means to have more than one evaluation per line of code. I also don't know that jobParams.keySet() returns a Set and that bugs me.
Another example would be:

//dislike
Bar.processParameter(Foo.getParameter());

vs

//like
Parameter param = Foo.getParameter();
Bar.processParameter(param);

The former makes me noxious and dizzy as I like to consume simple and clean evaluations in every line of code and I just hate it when I see other people's code written like that.

But are there any (performance) benefits to packing multiple method calls in the same line?

EDIT: Single liners are also more difficult to debug, thanks to @stemm for reminding

Upvotes: 6

Views: 2951

Answers (7)

Mike Dunlavey
Mike Dunlavey

Reputation: 40649

I don't see where a().b().c().d is that much harder to read than a.b.c.d which people don't seem to mind too much. (Though I would break it up.) If you don't like that it's all on one line, you could say

a()
 .b()
 .c()
 .d

(I don't like that either.) I prefer to break it up, using a couple extra variables. It makes it easier to debug.

If performance is your concern (as it should be), the first thing to understand is not to sweat the small stuff. If adding extra local variables costs anything at all, the rest of the code has to be rippin' fat-free before it even begins to matter.

Upvotes: 0

Eric B.
Eric B.

Reputation: 24411

I tend to disagree with most others on this list. I actually find the first way cleaner and easier to read.

In your example:

//like
Set<String> jobParamKeySet = jobParams.keySet();
Iterator<String> jobParamItrtr = jobParamKeySet.iterator();
Could be also written as

//dislike    
Iterator<String> jobParamItrtr = jobParams.keySet().iterator();

the first method (the one you like) has a lot of irrelevant information. The whole point of the iterator interface, for example, is to give you a standard interface that you can use to loop over whatever backing implementation there is. So the fact that it is a keyset has no bearing on the code itself. All you are looking for is the iterator to loop over the implemented object.

Secondly, the second implementation actually gives you more information. It tells you that the code will be ignoring the implementation of jobParams and that it will only be looping through the keys. In the first code, you must first trace back what jobParamKeySet is (as a variable) to figure out what you are iterating over. Additionally, you do not know if/where jobParamKeySet is used elsewhere in the scope.

Finally, as a last comment, the second way makes it easier to switch implementations if necessary; in the first case, you might need to recode two lines (the first variable assignment if it changes from a set to something else), whereas the second case you only need to change out one line.

That being said, there are limits to everything. Chaining 10 calls within a single line can be complicated to read and debug. However 3 or 4 levels is usually clear. Sometimes, especially if an intermediary variable is required several times, it makes more sense to declare it explicitly.

In your second example:

//dislike
Bar.processParameter(Foo.getParameter());
vs

//like
Parameter param = Foo.getParameter();
Bar.processParameter(param);

I find it actually more difficult to understand exactly which parameters are being processed by Bar.processParameter(param). It will take me longer to match param to the variable instantiation to see that it is Foo.getParameter(). Whereas the first case, the information is very clear and presented very well - you are processing Foo.getParameter() params. Personally, I find the first method is less prone to error as well - it is unlikely that you accidentally use Foo2.getParamter() when it is within the same call as opposed to a separate line.

Upvotes: 5

Durandal
Durandal

Reputation: 20059

The omission of an extra local variable probably has a neglible performance advantage (although the JIT may be able to optimize this).

Personally I don't mind call chaining when its pretty clear whats done and the intermediate object is very unlikely to be null (like your first 'dislike'-example). When it gets complex (multiple .'s in the expression), I prefer explicit local variables, because its so much simpler to debug.

So I decide case by case what I prefer :)

Upvotes: 1

Peter Lawrey
Peter Lawrey

Reputation: 533472

But are there any (performance) benefits to packing multiple method calls in the same line?

I seriously doubt the difference is measurable but even if there were I would consider

is hard for me to read the code.

to be so much more important it cannot be over stated.

Even if the it were half the speed, I would still write the simplest, cleanest and easiest to understand code and only when you have profiled the application and identified that you have an issue would I consider optimising it.

BTW: I prefer the more dense, chained code, but I would suggest you use what you prefer.

Upvotes: 1

Nandkumar Tekale
Nandkumar Tekale

Reputation: 16158

Code is never developed by same user. I would choose second way. Also it is easier to understand and maintain.

Also This is beneficial when two different teams are working on the code at different locations.

Many times we take an hour or more time to understand what other developer has done, if he uses first option. Personally I had this situation many times.

Upvotes: 1

kosa
kosa

Reputation: 66637

Micro optimization is killer. If the code references you are showing are either instance scope (or) method scope, I would go with second approach.

Method scope variables will be eligible for GC as soon as method execution done, so even you declare another variable, it's ok because scope is limited and the advantage you get will be readable and main-table code.

Upvotes: 6

Guido
Guido

Reputation: 47665

There is one less variable assignment, but even the compiler can optimize it in some cases. I would not do it for performance, it is kind of an early optimization. Write the code that is easier to maintain.

In my case, I find:

Iterator<String> jobParamItrtr = jobParams.keySet().iterator();

easier to be read than:

Set<String> jobParamKeySet = jobParams.keySet();
Iterator<String> jobParamItrtr = jobParamKeySet.iterator();

But I guess it is a matter of personal taste.

Upvotes: 3

Related Questions