Gotcha #48: Improperly Scoped Variables

One of the most common sources of bugs in C and C++ programs is uninitialized variables, and it's a problem that simply does not have to exist. Separating the declaration of a variable from its initializer rarely offers any advantage:

int a; 


a = 12;


string s;


s = "Joe";

That's just silly. The integer will have an indeterminate value until its assignment in the following statement. The string will be properly initialized by its default constructor but will be immediately overwritten by the following assignment (see also Gotcha #51). Both these declarations should have employed explicit initialization in the declaration-statement:

int a = 12; 


string s( "Joe" );

The real danger is that, under maintenance, code may be inserted between the uninitialized declaration and its first assignment. The typical scenario is a bit subtler than the code above:

bool f( const char *s ) { 


   size_t length;


   if( !s ) return false;


   length = strlen( s );


   char *buffer = (char *)malloc( length+1 );


   // . . .


}

Not only is length uninitialized, but it should be a constant. The author of this code has forgotten that in C++, as opposed to C, a declaration is a statement; to be precise, it's a declaration-statement, and a declaration can occur anywhere a statement can:

bool f( const char *s ) { 


   if( !s ) return false;


   const size_t length = strlen( s );


   char *buffer = (char *)malloc( length+1 );


   // . . .


}

Let's look at another common problem that generally occurs under maintenance. The following code is fairly unexceptional:

void process( const char *id ) { 


   Name *function = lookupFunction( id );


   if( function ) {


       // . . .


   }


}

The declaration of function is not too bad right now, but under maintenance, it can become a problem. As we mentioned earlier, maintainers will often reuse a local variable for a wildly different purpose. Why? Because it's there, I suppose:

void process( const char *id ) { 


   Name *function = lookupFunction( id );


   if( function ) {


       // process function . . .


   }


   else if( function = lookupArgument( id ) ) {


       // process argument . . .


   }


}

No bug yet, though I imagine the code for processing an argument is going to be pretty heavy going for the uninitiated reader ("In this section of the code, wherever I say 'function,' I mean 'argument.' ") But what happens when the original author comes back to do a little maintenance on function processing?

void process( const char *id ) { 


   Name *function = lookupFunction( id );


   if( function ) {


       // process function . . .


   }


   else if( function = lookupArgument( id ) ) {


       // process argument . . .


   }


   // . . .


   if( function ) {


       // postprocess function . . .


   }


}

Now we may attempt to postprocess an argument as a function.

It's usually best to restrict a name's scope to coincide precisely with where the original author intends that the name be used. Names still in scope but no longer used are a bit like unoccupied teenagers; they're just hanging out, waiting to get into trouble. The original function should have restricted the scope of the variable function to the scope of its intended use:

void process( const char *id ) { 


   if( Name *function = lookupFunction( id ) ) {


       // . . .


   }


}

Scoping the variable name removes the temptation to reuse it, and the eventual implementation of the function after maintenance will be more rational:

void process( const char *id ) { 


   if( Name *function = lookupFunction( id ) ) {


       // . . .


       postprocess( function );


   }


   else if( Name *argument = lookupArgument( id ) ) {


       // . . .


   }


}

C++ recognizes the importance of initialization and scoping of names. It provides a variety of language features to assist the programmer to ensure that every name is initialized and has scope corresponding precisely to its intended area of use.