Declare variables at first use.
Rationale:
It's best to declare variables when you first use them to ensure that they are always initialized to some valid value and that their intended use is always apparent. The alternative is typically to declare all variables in one location, typically at the top of the block or, even worse, at the top of a function. However, then the variable becomes separated from its meaning.
Since type information and scope are two of the most important pieces of information when dealing with a variable (in any language), and neither are apparent from the variable name (unless you use HungarianNotation), it's nice to have them apparent right there and then.
More importantly, though, by declaring the variable when it is first used, you give it context. You give it a reason to exist, which goes a long way towards understanding what the variable is for.
Arguments:
"It places variables all over my method/function! I can't find the declaration any more!"
The whole point is to make the search easier. If you're complaining because you find that you "lose" variable declarations in a long method/function, your method/function is too long. Cut it down. Use MethodsVsCodeFragments, NarrowTheInterface. Reduce the complexity. In general, a variable shouldn't be too far from where it is being used. (Corollary: stack frames should not be huge.)
"I like my variables all in one location, just like C. "
Unless you're using an old version of CeeLanguage, this is a broken idea. Each language is different for a reason. Use the language like it's supposed to be used. (This is a common case of BadCodingStandards.) However, if you're insistent, it's less important to find all variables before you read the code than to find variables as you read the code. That is, JustInTime learning. Moreover, you're more likely to have to initialize the variable to some garbage value, thus giving the variable no context from which to derive meaning.
Exceptions:
Clearly, in CeePlusPlus, it's impossible to declare member variables at first use, let alone initialize them at first use. In JavaLanguage, this is better, but you frequently end up initializing a variable to some junk value anyway. However, you could put all constructors (or a private Initialize() method that all constructors call) in the class definition in the header file. This can violate information hiding principles (ExtractImplementationFromHeader), but it does remind you to initialize your member variables.
In CeeLanguage prior to C99, you had to place all variables at the top of the block. Try to keep variables declared at the top of closest block, then. Also, in many cases one could easily initialize the variables with real values right there and then.
Examples:
Frequently, one sees this in CeeLanguage code:
{ int count; /* Counter */ ... for( count = 0; count < SomeMaximum; count++ ) ... }In CeePlusPlus (*) (and CeeLanguage as it stands today), one can do this:
{ ... for( int count = 0; count < SomeMaximum; count++ ) ... ... }which more tightly binds count's semantics to the for loop.
Notice, also, that the other style requires comments to make a variable's purpose clear. However, a variable has nothing to do with program flow. I'd rather follow flow than declarations.
Another example:
void CHedron::Paint() { CPoint ptScreen; // Screen projection of the world-coordinate point ... // Hey I'm distracting you by putting many lines between the declaration // and the actual initialization and use. ... // Whoops, ptScreen hasn't been initialized... suppose we try // using it in between declaration and initialization. Have fun // tracking that bug down. RotateAround( ptScreen, m_dYaw, m_dPitch, m_dRoll ); ... // What's your favourite colour? ... // TheRheostatics are the best band ever! ... CalculateLighting( this, GetWorld() ); ... ptScreen = Transform( GetOrigin(), g_WorldToScreenTransform ); ... // We can finally use ptScreen PaintAt( ptScreen ); }compared with
void CHedron::Paint() { // The above example tried this line before ptScreen was initialized // but this won't even compile if you uncomment it. // RotateAround( ptScreen, m_dYaw, m_dPitch, m_dRoll ); CalculateLighting( this, GetWorld() ); ... CPoint ptScreen = Transform( ptWorld, g_WorldToScreenTransform ); // ptScreen is always valid. No danger in using it unless there // is a *real* bug in the program logic, not just a silly mistake. RotateAround( ptScreen, m_dCurrentYaw, m_dCurrentPitch, m_dCurrentRoll ); ... PaintAt( ptScreen ); }
(*) Microsoft VisualCeePlusPlus (as of version 6) and many other compilers still can't create loop-scoped variables properly. That is, if you use while( int foo = ... ), foo's scope should only be the body of the while loop, but instead its scope is that of the enclosing block. The usual workaround for portable code is to write:
{ ... { while( int foo ) { ... } } ... }The reason the other means exist is to check the loop variable after the loop exits, as in:
for( int i = 0; i < SomeMax; < i++ ) { ... } if( i == SomeMax ) // Failed. ;Unfortunately, as you see, a whole scope has to be introduced to do this correctly, also creating a new indent level. This is gross. This wouldn't be so bad if MSVC++ wasn't the most popular compiler in the world. I'll bet they don't even put a compiler switch to be ISO-compliant because they have inline AFX/MFC (MicrosoftFoundationClasses) code that uses the broken idiom. I'll stop ranting now. ;) -- SunirShah
If you postpone declaring a variable until you know its value, very often it doesn't need to vary at all. It can be declared const or final. This reduces the amount of mutable state and so makes the code easier to understand. -- DaveHarris
For an example of SingleAssignment, check out the "paint" example at the end of ConstCorrectness (unless it's been moved to IntermediateValues).
An argument against this is that I find that having declarations mixed up everywhere distracts from the flow of the code. Perhaps this stems from my preference for DynamicTyping (and not declaring at all), but when reading code I am often not particularly interested in the type but what is being done with it. The variable name should sufficiently convey to me what it is. -- KristofferLawson
In JavaLanguage, you often cannot declare a variable where it is first used. The typical way to read from a stream is like this:
// No catch here--let the method throw IOException. // The caller can handle the problem. BufferedReader dictionaryReader = null; try { dictionaryReader = new BufferedReader(new FileReader(DICT_NAME)); // Read stuff } finally { if (dictionaryReader != null) { try { dictionaryReader.close()) } catch (IOException ignored) { // What do I do now? } } }Setting the reader to null in the beginning appeases the compiler; otherwise, javac complains about possibly uninitialized variables. The reader cannot be declared inside the try block, as it would be out of scope in the finally block. -- EricJablow
No, you should write it like this instead:
// No catch here--let the method throw IOException. // The caller can handle the problem. // If exception is thrown from the constructor, you won't be closing the reader anyway. BufferedReader dictionaryReader = new BufferedReader(new FileReader(DICT_NAME)); try { // Read stuff } finally { try { dictionaryReader.close()) } catch (IOException ignored) { // What do I do now? } }
See also: SelfDocumentingCode