Calling getters on an object vs. storing it as a local variable (memory footprint, performance)

Question

In the following piece of code we make a call listType.getDescription() twice:

for (ListType listType: this.listTypeManager.getSelectableListTypes())
{
    if (listType.getDescription() != null)
    {
        children.add(new SelectItem( listType.getId() , listType.getDescription()));
    }
}

I would tend to refactor the code to use a single variable:

for (ListType listType: this.listTypeManager.getSelectableListTypes())
{
    String description = listType.getDescription();

    if (description != null)
    {
        children.add(new SelectItem(listType.getId() ,description));
    }
}

My understanding is the JVM is somehow optimized for the original code and especially nesting calls like children.add(new SelectItem(listType.getId(), listType.getDescription()));.

Comparing the two options, which one is the preferred method and why? That is in terms of memory footprint, performance, readability/ease, and others that don't come to my mind right now.

When does the latter code snippet become more advantageous over the former, that is, is there any (approximate) number of listType.getDescription() calls when using a temp local variable becomes more desirable, as listType.getDescription() always requires some stack operations to store the this object?

Answer 1

I'd nearly always prefer the local variable solution.

Memory footprint

A single local variable costs 4 or 8 bytes. It's a reference and there's no recursion, so let's ignore it.

Performance

If this is a simple getter, the JVM can memoize it itself, so there's no difference. If it's a expensive call which can't be optimized, memoizing manually makes it faster.

Readability

Follow the DRY principle. In your case it hardly matters as the local variable name is character-wise as about as long as the method call, but for anything more complicated, it's readability as you don't have to find the 10 differences between the two expressions. If you know they're the same, so make it clear using the local variable.

Correctness

Imagine your SelectItem does not accept nulls and your program is multithreaded. The value of listType.getDescription() can change in the meantime and you're toasted.

Debugging

Having a local variable containing an interesting value is an advantage.

---

The only thing to win by omitting the local variable is saving one line. So I'd do it only in cases when it really doesn't matter:

• very short expression
• no possible concurrent modification
• simple private final getter

Answer 2

I think the way number two is definitely better because it improves readability and maintainability of your code which is the most important thing here. This kind of micro-optimization won't really help you in anything unless you writing an application where every millisecond is important.

Answer 3

I'm not sure either is preferred. What I would prefer is clearly readable code over performant code, especially when that performance gain is negligible. In this case I suspect there's next to no noticeable difference (especially given the JVM's optimisations and code-rewriting capabilities)

Answer 4

In the context of imperative languages, the value returned by a function call cannot be memoized (See http://en.m.wikipedia.org/wiki/Memoization) because there is no guarantee that the function has no side effect. Accordingly, your strategy does indeed avoid a function call at the expense of allocating a temporary variable to store a reference to the value returned by the function call. In addition to being slightly more efficient (which does not really matter unless the function is called many times in a loop), I would opt for your style due to better code readability.

Answer 5

I agree on everything. About the readability I'd like to add something: I see lots of programmers doing things like:

if (item.getFirst().getSecond().getThird().getForth() == 1 ||

item.getFirst().getSecond().getThird().getForth() == 2 ||

item.getFirst().getSecond().getThird().getForth() == 3)

Or even worse:

item.getFirst().getSecond().getThird().setForth(item2.getFirst().getSecond().getThird().getForth())

If you are calling the same chain of 10 getters several times, please, use an intermediate variable. It's just much easier to read and debug

Answer 6

I would agree with the local variable approach for readability only if the local variable's name is self-documenting. Calling it "description" wouldn't be enough (which description?). Calling it "selectableListTypeDescription" would make it clear. I would throw in that the incremented variable in the for loop should be named "selectableListType" (especially if the "listTypeManager" has accessors for other ListTypes).

The other reason would be if there's no guarantee this is single-threaded or your list is immutable.