views:

71

answers:

3

I would like to know if there are any significant disadvantages to creating a temporary variable for a value that is accessed multiple times within a small scope.

eg:

filep->waypt[rp->leg[j]].ID

or

(*(filep->route + filep->nroutes - 1))->number

These are examples from a recent project where I forced myself to avoid almost any simplification for variable references in order to improve my skills with C pointers (it was painful). However, the habit seemed to stick. I find that the more I try to introduce variables to simplify my code (readability and typing volume) the more confusing it becomes to remember exactly what each new variable references.

I've only ever had my code reviewed in an educational setting, and I would like to know what others find easier to digest and where are the performance tradeoffs (if any).

When would assigning a value who's memory address would require several arithmetic operations to calculate it's own variable start to cause performance issues? Would it make a difference if done once in a loop? What about once in a nested loop? What about a value assigned to it's own variable in a loop, but is then accessed in an inner loop?

How would this change in an interpreted language? Say in PHP (please excuse syntactical errors, I'm new to PHP):

$employees[$i][$phone]['Home']['number'];

vs

$home = $employees[$i][$phone]['Home']['number'];

And finally, if it's not too subjective (code readability shouldn't be!), which is considered the best practice for writing readable code? I write code that is as self-documenting as possible, but if variables begin to get too contrived and are referenced multiple times, I will assign them their own variable. However, the reason this works for me could be that I've gotten used to my own style.

+5  A: 

There's a fairly good chance that any decent compiler will detect common sub-expressions and optimise them for you.

However, I would still opt for a temporary variable if it improved the readability of my code, despite the fact that it introduced extra storage (though minimal).

I consider maintainability of code far more important than a tiny storage hit. By way of (contrived) example, I would probably replace a whole lot of:

item[last_item-1].id = 14860;
item[last_item-1].name.first = "pax";
item[last_item-1].name.last = "diablo";

calls with:

tItem lastItem = &(item[last_item-1]);
lastItem->id = 14860;
: : :
// and so on.

As to interpreted languages, I have less information on that. I know quite a bit about how to optimise assembly code (though nowhere near as much as the demigods that write compilers, which is why I normally leave it to them). I know somewhat less about the virtual machines running inside most interpreters.

But I would still code for readability. I don't want to see a bucketload of $employees[$i][$phone]['Home']['number'] snippets stuck in my code. I would rather add

$homePhone = $employees[$i][$phone]['Home']['number'];

in there somewhere and just use $homePhone from that point onward.

paxdiablo
Without performance profiling it's hard to say that the temporary variable approach is worse. Often walking a big tree of references (i.e 4 array indexes above) can have a bigger performance impact as opposed to creating a local cache of the lookup.
Toby
+1 for "I consider maintainability of code far more important than a tiny storage hit."
David Stratton
Toby: yes, it all depends on the compiler. It will (if it's smart) remember the `item[last_item-1]` for the second and third assignments (`name.first` and `name.last` will most likely be constant offsets but, even if it were `.name->first`, it could still remember the `.name` bit). My point was: code for readability, only tackle performance if it turns out to be a problem.
paxdiablo
It would seem that the general consensus is code for readability at all costs and optimize as required. Although $employees[$i][$phone]['Home']['number']in a larger loop seems to me more self explanatory than $homePhone = $employees[$i][$phone]['Home']['number'];I can definitely understand why the shorter variable would be preferred.All the answers were very informative into the desire for readable code!
Eric Coutu
A: 

I would strongly encourage you to use variables for values that are reused at all. The key is to use variable names that actually describe what your are using the value for or what the value is. Also using a variable to store the value and then using the variable makes modification waaay easier in the future and much less error prone.

Here are some naming resources as well to help you with naming variables well. Also don't be afraid to comment your code so you know what each variable, function, class, etc.. was intended to do. Microsofts Naming guidelines

Stackoverflow Post on PHP naming guidelines

UPDATE: removed my example because it was wrong as pointed out in the comment below. I would still recommend using variables for values that are reused within the function as it improves readability and maintainability.

antonlavey
The above C# code will not 'Count' the items in your array more than once. The compiler will deal with this and your probably shooting yourself in the foot by removing possible optimisations from the JIT's aresenal (see http://blogs.msdn.com/b/ericgu/archive/2004/04/18/115566.aspx). This along with the fact that arrays don't have a Count, but a Length which when accessed does not 'Count' the number of items in the array.
Toby
You are right was thinking of lists. edited on that. Didn't realize that the C# compiler was smart enough to split that off properly. I do know that in PHP the example held true but I removed it to avoid confusion. Learn something new every day thanks.
antonlavey
A: 

Use them for readability and also when you know that calling the method that returns the value for the temporary variable is expensive (e.g. when you request the credit score from an external agency you may incur both performance hit and costs).

In some cases I not only introduce temporary (aka local) variables for better readability but I sometimes even extract methods (through refactoring tools support) for a single line of code if the method name makes it easier to understand what the code is about.

I look into performance if and only if the system already behaves correctly but not is not fast enough yet. That way I avoid optimization of code that is not yet in its final form.

John