Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

I have an old habit of avoiding calling references multiple times, both for easier to read/maintain code, and for possible efficiency. But I'm wondering which is more efficient (memory, performance, both?).

For example when working with XML using a VTD parser

if( vn.toString( vn.getCurrentIndex() ).equalsIgnoreCase( "broken-label" ) == false )
        {
            do
            {   
                if( parentNode != null )
                {
                    currentNode = parentNode.addChildNode( vn.toString( vn.getCurrentIndex() ) );
                }
                else
                {
                    currentNode = new xmlNode( vn.toString(vn.getCurrentIndex()), null );
                    treeNodes.add( 0, currentNode );
                }

Does not store the value, perhaps saving some overhead for creating space to save a local variable and also lowering the burden on the garbage collector (assuming this section of code is repeated thousands of times in quick succession.

My habit of cleaner/efficient code would be to replace the above with the simple change of

String label = vn.toString( vn.getCurrentIndex();

if( label ).equalsIgnoreCase( "vsled-image" ) == false )
        {
            do
            {   
                if( parentNode != null )
                {
                    currentNode = parentNode.addChildNode( label ) );
                }
                else
                {
                    currentNode = new xmlNode( label, null );
                    treeNodes.add( 0, currentNode );
                }

While this is obviously easier to read and maintain. Are there any non-human benefits?

share|improve this question
2  
Your cut and paste from the conditional to the assignemnt in your second example appears to have left stray ) and ( in various places that you might want to clean up. Your indenting is inconsistent (and deep) - you might want to consider sending your code through prettyprinter.de (might change from your style, but it will make it consistent. As an aside, if(something == false) makes me sad (try if(!something) instead). This becomes possibly important if someone did something silly like new Boolean("false") and the autoboxing is done on the wrong operand) –  MichaelT Oct 16 '13 at 1:55
    
There is additional code below that was not pertinent to the question so and I didn't feel like fighting with the text box here to add properly indented braces though yes there's some missing parenthesis as well, this was meant more as pseudo code to demonstrate the idea. Some programmers I work with miss "!" and think the statement was checking for true so I go with the more visible false to prevent common human errors on the current project. –  FaultyJuggler Oct 16 '13 at 16:54
    
I've found that prettyprinter.de does a reasonable job of formatting code for pasting into a text box. Significant syntactical errors unmatched parens) can often distract from the meaning of the message itself and have people (myself for example focus on the wrong part of the question. As to the == comparison, well, until you've been hit with such an auto boxing bug it can be difficult to appreciate the difficulty of tracking it down. Consider on SO stackoverflow.com/questions/3882095 and stackoverflow.com/questions/4045682 where autoboxing bugs bite. –  MichaelT Oct 16 '13 at 17:05
add comment

3 Answers

up vote 0 down vote accepted

Function calls are expensive. In this example, by using that variable, you have eliminated four separate function calls. Each call puts stuff on the call stack, jumps the instruction pointer around, creates the rough equivalent of brand new variables anyway, etc. On many occasions, functions you call will create what really are new variables within their definitions as well.

So, yeah, in cases like this, it's better to just go ahead and make that extra variable up at the top. And if you're worried to death about it, and you're willing to sacrifice modularity and maintainability, you might set up the variable outside of the function's body, before the function is called, and then just reference it within the function.

share|improve this answer
1  
This is misleading... Methods calls are not necessarily expensive in Java. –  assylias Oct 15 '13 at 22:25
    
Why would you increase the scope of the variable if you don't need to? If you put it out of the function you now have needlessly introduced state on the object you have to take care of (and test, debug). –  TheMorph Oct 15 '13 at 23:08
    
I generally wouldn't. I said, "And if you're worried to death about it, and you're willing to sacrifice modularity and maintainability..." This was in response to him mentioning the same overall code (and probably overall function) getting executed over and over. –  Panzercrisis Oct 16 '13 at 12:36
    
If these method calls are simple and inexpensive to run they'll likely end up getting inlined anyway--function calls don't have to be costly. But that's beside the point, we're talking about "four separate function calls". Being concerned about the performance of calling four functions is so far beyond the realms of premature optimization that even considering it seems absurd to me. Now, if those methods themselves are non-trivial, the result should be cached--but that's a completely different question. –  Phoshi Oct 16 '13 at 12:42
    
He wasn't getting into whether this were premature optimization or not though...But I agree with what he was saying earlier, which was that by storing this in a variable, it can be more human-readable. When something is easier to read and write, I can't really call that premature optimization, unless somebody spends all day obsessing over something like this. Again, I would almost never recommend taking that variable outside of its present scope, as I was just mentioning that for bizarre cases. –  Panzercrisis Oct 16 '13 at 13:04
show 1 more comment

If the methods are simple getters it makes no difference as they will be inlined at runtime.

If the methods do non trivial work, then calling them more often will probably take more time. To know exactly how much, you will need to profile your application.

From a readability perspective, using well chosen variables may improve your code.

share|improve this answer
    
is .toString() trivial here? I've dealt with certain architectures where repeated reference calls are more expensive than temporarily holding onto the value to work with. I just don't know enough about low level Java mechanics to know for sure. –  FaultyJuggler Oct 16 '13 at 16:58
    
@FaultyJuggler that toString() could be as simple as return someString; or it could be as terrible as String result = ""; for (String t: someHugeList) result += t;... Hence my answer: whether performance will be different or not depends. But using temporary local variables won't hurt performance - so if it is more readable too it is a no brainer. –  assylias Oct 16 '13 at 17:03
    
Yeah: "It's easier to optimize correct code than to correct optimized code." For this kind of decision, focus on making the code clear, and profiling will tell you what to bother changing, if anything. –  Darien Oct 16 '13 at 20:29
add comment

One point not mentioned here: vn.getCurrentIndex() might get a different value with each call. Your local variable will retain its value, which can be quite useful.

(Another thread might change it. Or a trivial-looking method that obviously has nothing to do with the current index might call something that calls something that ... changes the index.)

share|improve this answer
    
This is single threaded and the VtdNav (in this section of code at least) is guaranteed to keep the same index, but a valid point. If I threaded it I'd have each thread keep it's own VtdNav however and focus on one file per thread. –  FaultyJuggler Oct 16 '13 at 21:01
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.