Declare variables at top of function or in separate scopes?

Question

Which is preferred, method 1 or method 2?

Method 1:

LRESULT CALLBACK wpMainWindow(HWND hwnd, UINT msg, WPARAM wparam, LPARAM lparam)
{
    switch (msg)
    {
        case WM_PAINT:
        {
            HDC hdc;
            PAINTSTRUCT ps;

            RECT rc;
            GetClientRect(hwnd, &rc);           

            hdc = BeginPaint(hwnd, &ps);
            // drawing here
            EndPaint(hwnd, &ps);
            break;
        }
        default: 
            return DefWindowProc(hwnd, msg, wparam, lparam);
    }
    return 0;
}

Method 2:

LRESULT CALLBACK wpMainWindow(HWND hwnd, UINT msg, WPARAM wparam, LPARAM lparam)
{
    HDC hdc;
    PAINTSTRUCT ps;
    RECT rc;

    switch (msg)
    {
        case WM_PAINT:
            GetClientRect(hwnd, &rc);

            hdc = BeginPaint(hwnd, &ps);
            // drawing here
            EndPaint(hwnd, &ps);
            break;

        default: 
            return DefWindowProc(hwnd, msg, wparam, lparam);
    }
    return 0;
}

In method 1, if msg = WM_PAINT when wpMainWindow function is called, does it allocate memory for all the variables on the stack at the beginning? or only when it enters the WM_PAINT scope?

Would method 1 only use the memory when the message is WM_PAINT, and method 2 would use the memory no matter what msg equaled?

Solution 1

Variables should be declared as locally as possible.

Declaring variables "at the top of the function" is always a disastrously bad practice. Even in C89/90 language, where variables can only be declared at the beginning of the block, it is better to declare them as locally as possible, i.e. at the beginning of smallest local block that covers the desired lifetime of the variable. Sometimes it might even make sense to introduce a "redundant" local block with the only purpose of "localizing" the variable declaration.

In C++ and C99, where it is possible to declare variable anywhere in the code, the answer is pretty straightforward: again, declare each variable as locally as possible, and as close as possible to the point where you use it the very first time. The primary rationale for that is that in most cases this will allow you to supply a meaningful initializer to the variable at the point of declaration (instead of declaring it without initializer or with a dummy initializer).

As for the memory usage, in general a typical implementation will immediately (as you enter the function) allocate the maximum space required for all variables that exist at the same time. However, your declaration habits might affect the exact size of that space. For example, in this code

void foo() {
  int a, b, c;

  if (...) {
  }

  if (...) {
  }
}

all three variables exist at the same time and generally the space for all three has to be allocated. But in this code

void foo() {
  int a;

  if (...) {
    int b;
  }

  if (...) {
    int c;
  }
}

only two variables exist at any given moment, meaning that space for only two variables will be allocated by a typical implementation (b and c will share the same space). This is another reason to declare variables as locally as possible.

Solution 2

Whether something's allocated on the stack in case 1 is implementation defined. Implementations aren't even required to have a stack.

It's usually no slower to do so since the operation tends to be a simple subtraction (for a downward growing stack) of one value from the stack pointer for the entire local variable area.

The thing that's important here is that the scope should be as local as possible. In other words, declare your variables as late as possible and only keep them around as long as needed.

Note that declaring here is at a different abstraction level to allocating space for them. The actual space may be allocated at the start of the function (implementation level) but you can only use those variables while they're scoped (C level).

Locality of information is important, just like its cousin, encapsulation.

Solution 3

I like Method 3:

LRESULT wpMainWindowPaint(HWND hwnd)
{
    HDC hdc;
    PAINTSTRUCT ps;

    RECT rc;
    GetClientRect(hwnd, &rc);           

    hdc = BeginPaint(hwnd, &ps);
    // drawing here
    EndPaint(hwnd, &ps);
    return 0;
}

LRESULT CALLBACK wpMainWindow(HWND hwnd, UINT msg, WPARAM wparam, LPARAM lparam)
{
    switch (msg)
    {
        case WM_PAINT:      return wpMainWindowPaint(hwnd);
        default:            return DefWindowProc(hwnd, msg, wparam, lparam);
    }
}

If it deserves its own scope for organization purposes, it deserves its own function. If you're worried about function call overhead, make it inline.

Solution 4

Since it's the compiler's job to optimize my code, and an hour of compiler-time is way cheaper than an hour of my time, and my time gets wasted if I need to scroll up and down the code to see where a variable was declared, I think my company wants me to keep everything as local as possible.

Not even am I talking about 'the smallest block', but 'as near to the place where it is used'!

LRESULT CALLBACK wpMainWindow(HWND hwnd, UINT msg, WPARAM wparam, LPARAM lparam) 
{ 
    switch (msg) 
    { 
        case WM_PAINT: 
        { 
            RECT rc; 
            GetClientRect(hwnd, &rc);            

            { // sometimes I even create an arbitrary block 
              // to show correlated statements.
              // as a side-effect, the compiler may not need to allocate space for 
              // variables declared here...
              PAINTSTRUCT ps; 
              HDC hdc = BeginPaint(hwnd, &ps); 
              // drawing here 
              EndPaint(hwnd, &ps); 
            }
            break; 
        } 
        default:  
            return DefWindowProc(hwnd, msg, wparam, lparam); 
    } 
    return 0; 
} 

Solution 5

Define the variables in the narrowest scope where they are relevant. There's no reason to use Method 2 above in my opinion.

Stack space is only likely to be used when the variables are in scope. As @paxdiablo points out, your locals may wind up in registers rather than on the stack, if the compiler can find the space for them.

View more solutions

Comments

[AnT stands with Russia]

There are two kinds of C in active use today: C89/90 and C99. They differ greatly with respect to where variables can be declared.

[Jonathan Leffler]

@AndreyT: you're right, of course, but the code shown is fine with C89 or C99...

[Ben Voigt]

If you keep your functions to a reasonable complexity, there's isn't enough difference to worry about.

[Roman Nikitchenko]

stackoverflow.com/questions/1688241/… is very similar in many ways and you can see my answer there.

[leetNightshade]

@Roman Nikitchenko Not really. This is about preference, not necessarily for the benefit of the compiler. Sure there end up being similarities to the best practice, but it's not the same. If you only look at the 2 methods, it makes it seem similar, until you see Method 3 given by Ben Voigt. Adding a function that can't be inlined by the compiler doesn't seem more optimal, but it's a great solution, a good design decision for practicality.

[Kaije]

thanks, i usually do make them local like in my method 1, but i have been reading a window programming book, and they do everything like in method 2. but hte book is from 1998....

[Ben Voigt]

@Steve: Standard C has always allowed variable declarations at the beginning of any block, not just at the top of the function.

[flies]

there's a slight conflict between "declare as locally as possible" and efficiency when you're declaring variables inside a loop. My tendency has been to declare variables used for a large loop prior to the loop - is this bad practice? (assume that the variables have no use/meaning outside the loop.)

[TheUndeadFish]

@flies As with most recommendations, there follows an unstated: "unless you have a good reason to do otherwise in your situation". Efficiency might be enough of a reason to move variables outside of a loop in some cases. (Though for primitive types it probably doesn't make much of a difference in most cases.) And you could still surround the variables and loop with another scope to keep them from leaking into anywhere else, thus following the intent of the original idea.

[paxdiablo]

@flies, the implementation does not have to do anything when you declare a variable inside a non function block. An implementation may know all the variable space that will be required by the function and allocate it at the start. There's a disconnect between what the implementation can do and what your code can do - access to the variable by your code can be controlled by the compiler separately from the space allocation the variable. In fact, a compiler is even free to detect lack of recursion and allocate space for all variables at program start :-)

[JeremyP]

@AndreyT: Declaring variables at the top of the function is not always a disastrously bad practice. In fact, some people argue it is a good practice because you can always find them easily. Personally, I would recommend doing exactly as you say in your answer but the World will not end if you use the alternative.

[AnT stands with Russia]

@JeremyP: In my experience that was actually one argument against declarations at the top: it is much more difficult to find the declaration when all declarations are piled up at the top. Yes, it might be easier to find the pile itself, but trying to find a specific declaration in it is just nearly impossible.

[flies]

@TheUndeadFish @paxdiablo thanks for your helpful and informative responses.

[JeremyP]

@AndreyT: The argument was usually on the basis that with large functions, having variable declarations buried within them made the declarations harder to find (ignoring the point that it means the function is probably too large). I'm just saying that your characterisation of "disastrously bad" was over the top.

[AnT stands with Russia]

@JeremyP: I called it disastrously bad because the list of cons I can think of is much much longer than the list of pros. And one of the cons is that this declaration style encourages variable reuse, leading to truly disastrously bad code.

[marcus]

@flies If the language is C++, that will depend on the amount of work done by the constructor and operator=. Declaring and initializing calls the [copy] constructor (even if the initialization is done with an = sign) and assigning (outside of a declaration) calls operator=. So declaring an object outside of the loop can be worse because it creates a dummy object that will be overwritten. OTOH if you are working with primitives, ptrs, refs or plain C there's no need to worry because the declaration itself generates little or no code at all: what really generates code is what's after the = sign.