March 13, 2006

Initializing variables

While reviewing the updatable views patch that Bernd and Jaime recently posted (more on that later), I ran into a few coding patterns that annoy me. One of those is the pointless initialization of block-local variables, as in:

T *
foo_func(...)
{
    T *v = NULL;
    
    v = palloc(sizeof(*v));
    /* initialize v */
    
    return v;
}

Some programmers are in the habit of initializing all block-local variables when they define them — even in the example above, where the initial value of v is never referenced. So, is it good style to always initialize v when it is defined? IMHO, no.

I suppose the justification for this technique is that it ensures that v always has a well-defined value: if we didn’t initialize v, it would initially be undefined. At first glance, that sounds reasonable: dereferencing an undefined variable can have unpredictable consequences, whereas dereferencing the NULL pointer is guaranteed to halt the program.

I think this reasoning is flawed for a few reasons:

  1. It hides errors: if you leave v undefined and then try to use its value before it has been assigned to, most modern compilers will warn you about your mistake: obviously, reading the value of an undefined variable is probably not what the programmer intended to do. If you initialize v to NULL, the compiler can’t tell this, which makes this error harder to detect. (While you won’t get a compiler error in any case, if you are ignoring or not enabling compiler warnings you are probably a bad programmer in any case.)

  2. It is confusing: the only reason to initialize v is if its value might be used without any subsequent assignment. That is, this is perfectly reasonable code:

    T *
    foo_func(...)
    {
        T *v = NULL;
        
        if (some_condition)
        {
            v = palloc(sizeof(*v));
            /* initialize v */
        }
        
        return v;
    }
    

    In this example, initializing v to NULL indicates that there are some code paths where the initial value of v will not be overwritten. This is a valuable hint, because it tells you something non-trivial about the way in which v is used. By adding a pointless initialization of v in the first example, it makes it more difficult to distinguish between these two cases for no gain.

  3. It is redundant: There’s no need for the initialization, so by Ockham’s razor we ought to eliminate it: if you can remove a construct from your code without harming readability, functionality or performance, you probably should.

    (An explicit initialization might also cause the compiler to generate more code, but a decent optimizing compiler should usually be able to recognize that the initial value of v is not needed and thus can be eliminated — and in any case the performance difference is unlikely to be significant.)

If anyone can provide a good justification for this coding pattern, I’d be curious to hear it.

Advertisements

Leave a comment

Filed under Advogato, PostgreSQL

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s