java pattern: when does it make sense to use temporary variables

Question

So I find myself doing something like the following pattern often. Instead of:

if (map.containsKey(someKey)) {
    Value someValue = map.get(someKey);
    ...
}

To not traverse the map twice (and since I know my map doesn't store nulls), I'll do:

Value someValue = map.get(someKey);
if (someValue != null) {
    ...
}

This seems to me to be a worthwhile pattern given that the Map operations perform a decent number of operations and the optimizer, I assume, is not smart enough to optimize it away.

But then I find myself doing similar patterns in other situations. For example, should I store the someMethod() result in a temporary variable instead of making the call twice? Clearly I can't call someMethod() twice if there are side-effects but when does it make sense to only call it once from an optimization standpoint?

if (someObject.someMethod() != null) {
    processSomehow(someObject.someMethod());
}

I know this borders on a "not constructive" question so I'm looking for answers that present some facts and references instead of just conjecture.

• When does it make sense to do this and when does it not?
• How should I evaluate the "cost" of the someMethod() to determine when a temporary variable should be used?
• For simple methods such as get methods, might this get in the way of the hotspot compiler or the optimizer and actually produce less efficient code?

For posterity, I am not asking "how can I improve the speed of my existing program". I am trying to figure out "what pattern should I use when I write future code".

Thanks for any information.

Answer 1

How should I evaluate the "cost" of the someMethod() to determine when a temporary variable should be used?

Simply look at the implementation of someMethod(). If it just returns a field, like a typically getter method would do, there's no need to declare a local variable (performance wise). If the method creates objects, calls the database or reads file contents, then is usually wise to cache the return value.

Personally, I don't care of performance issues caused be declaring a temporary variable. I just keep the return value instead of calling a method twice. The code is much easier to read and to understand (unless you name all those variables just temp;-) )

Answer 2

Clearly I can't call someMethod() twice if there are side-effects but when does it make sense to only call it once from an optimization standpoint?

When you've proved that it matters using a profiler. But get used to not calling methods twice in a row anyway, since it will make maintenance easier (what if the check changes and you forget to change it in both places?). Use temporary variables liberally, they're incredibly cheap.

Answer 3

As a general rule I use a local var when I need it more than once. You can give that local var a pretty name and everybody knowns what this var is about. From performance point of view: Local vars are cheap and methods maybe complex.

Especially when someMethod() is expensive like a synchronized method then this should perform much better. But when someMethod() is a synchronized method then it is intended to be called by multiple threads in concurrency and then it makes a difference to call the method twice or store its return value and reuse it ...

...Another point to mention is that subsequent method calls do not have to return the same data. So your examples with/without local var are not always valid substitutes for subsequent method calls. But I think you already considered this.

Answer 4

For me this all boils down to the logic of the program and not to performance. So

if (map.containsKey(someKey)) {
    Value someValue = map.get(someKey);
    ...
}

is not equivalent to that code in all cases:

Value someValue = map.get(someKey);
if (someValue != null) {
    ...
}

Consider a map where the value can be null for a given key. If the ... part is null safe then the code behaves differently.

Also this code with somehow has logical issues. Consider this:

BufferedRead reader = ...;
if (reader.readLine() != null) {
    processSomehow(reader.readLine());
}

Here it is clear that readLine must be called twice. Another problem domain for this kind of code is multithreading: The code expects the value not to change, but another thread might do excatly this.

Summary Keep your code as crisp and correct as possible. This means, that if you expect a value not to change and you need it multiple times, then use a variable to tell anybody this.

Answer 5

Several people are quoting Knuth (either implicitly or explicitly), but I think this always makes sense as a design pattern (and cases where null is an allowable key, as pointed out by A.H. become that much more clear).

In my mind, it's similar to the case of passing a variable by const reference in C++ instead of by value. There are two reasons to do this:

1. it communicates that the value won't change
2. but the main reason it's done is that it's marginally faster.

It doesn't matter on most individual calls, but when every routine passes every parameter by value, you notice it. It's a good habit to be in. The case you mention isn't as common and so it isn't as important, but I'd argue it's still a good habit.

Answer 6

Reminds me of a discussion regarding similar .NET Dictionary usage style: What is more efficient: Dictionary TryGetValue or ContainsKey+Item?

In general, as pointed out in comments, you don't know the performance hot spots in your program until you run it with real data under a profiler.

Also, in general, compilers in imperative languages can not optimize repeated calls, because of the possible side effects. Every call to a function can return a new value. And by the way, that is also one of the reason why you yourself shouldn't rely on repeated calls. Today maybe you know that a function or a property is side-effect free and repeated calls return the same value, but tomorrow you change it and add some random generator inside and all logic with repeated calls will be broken.