Don't name your variables 'temp'
You know what? Just don’t name your variables temp. It’s an incredibly bad practice (no, I don’t have OCD). I saw many variations of usage of temp variables in all kinds of situations. But maybe I’m just unlucky. Let me show you some examples:
Justified case (pseudo C#)
That’s the only type of problem I can think of when naming variable temp is acceptable. Let’s jump to some horror stories.
The horror (pseudo C)
This should roughly get you an idea what this abomination of a function looked like. It was meant to grab a single flag from the serial port. The flag indicated what data is coming next on the port. Since you know what type of data you are going to receive you should read as many bytes as you need for the given type. Then the code was performing some magic calculations to finally put the correctly formatted value to some struct or global variable.
The function was around 600 lines long. My biggest issue with the code was that the temp variables were used all over the function and they were not simply used as containers for holding the values, they were often modified in different places and assigned to different variables.
Clearly the design above is flawed. Some people might argue that it’s worth to reuse variables this way to save memory. I disagree for two reasons:
- Readability of the code is way more important than performance. If you are worried about the performance then code it properly first and then try to optimize your code if you run into performance issues.
- The compiler should be quite good when dealing with scopes. If you declare your variable within an if statement and the condition for this statement is not met then the variable should not be constructed, saving you some space.
How would I fix the code?
Depending on the amount of time I had I would:
- Do a heavy refactoring, create lots of smaller functions to handle each received item independently
- I would substitute temp variables with lots of meaningful variable names, then having more clear idea what variable is accessed where I would try to limit their scope as much as possible (ideally to the single cases in switch statement).
Second horror story
This code… It’s real and it’s out there somewhere being executed right now. All the temp values were used in some matrix calculations. Simply don’t do that. If you don’t know what’s wrong about this code then go and read Clean Code, if you still don’t know why it’s bad then read Code Complete. If you still don’t understand what’s wrong with this approach then mail me, I want to meet you.
Words of wisdom
Here is what Steve McConnell says on temp variables in Code Complete (chapter 10.8):
Use each variable for one purpose only. It’s sometimes tempting to use one variable in two different places for two different activities. Usually, the variable is named inappropriately for one of its uses or a “temporary” variable is used in both cases (with the usual unhelpful name x or temp).
Do your best to give meaningful names to the variables. If you can’t then at least substitute every occurrence of ‘temp’ with unicorn, at least it will be more cute that way.
What happens if you keep giving meaningless names to your variables?