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?

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. — AnT stands with Russia, Sep 22 '10 at 20:47
@AndreyT: you're right, of course, but the code shown is fine with C89 or C99... — Jonathan Leffler, Sep 22 '10 at 20:57
If you keep your functions to a reasonable complexity, there's isn't enough difference to worry about. — Ben Voigt, Sep 22 '10 at 21:42
http://stackoverflow.com/questions/1688241/when-is-stack-space-allocated-for-local-variables is very similar in many ways and you can see my answer there. — Roman Nikitchenko, Sep 23 '10 at 07:00
@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. — leetNightshade, Feb 19 '14 at 21:39

AnT stands with Russia · Accepted Answer · 2014-12-03T18:34:59.027

91

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.

edited Dec 03 '14 at 18:34

answered Sep 22 '10 at 20:43

AnT stands with Russia

312,472
42
525
765

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.... – Kaije Sep 22 '10 at 20:58
@Steve: Standard C has always allowed variable declarations at the beginning of any block, not just at the top of the function. – Ben Voigt Sep 22 '10 at 21:35
2

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.) – flies Sep 22 '10 at 21:36
5

@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. – TheUndeadFish Sep 22 '10 at 22:18
1

@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 :-) – paxdiablo Sep 22 '10 at 23:59
1

@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. – JeremyP Sep 23 '10 at 09:39
1

@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. – AnT stands with Russia Sep 23 '10 at 13:46
@TheUndeadFish @paxdiablo thanks for your helpful and informative responses. – flies Sep 23 '10 at 14:28
@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. – JeremyP Sep 24 '10 at 08:39
9

@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. – AnT stands with Russia Sep 24 '10 at 13:42
2

@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. – marcus Apr 21 '13 at 17:09

score 13 · Answer 2 · edited Sep 22 '10 at 20:54

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.

score 7 · Answer 3 · answered Sep 22 '10 at 21:40

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.

xtofl · Answer 4 · 2010-09-22T21:26:01.093

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; 
}

Steve Townsend · Answer 5 · 2010-09-22T20:48:11.160

3

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.

edited Sep 22 '10 at 20:48

answered Sep 22 '10 at 20:42

Steve Townsend

53,498
9
91
140

score 1 · Answer 6 · answered Jul 13 '12 at 02:58

For Java programming language, the common practice is to declare local variables only when needed in a method.

void foo(int i) {
  if (i == 1)
    return;
  Map map1 = new HashMap();
  if (i == 2)
    return;
  Map map2 = new HashMap();
}

For C++ programming language, I also suggest the same practice since declaring variables with a non-trivial constructor involves execution cost. Putting all these declarations at the method beginning causes unneeded cost if some of these variables will be used.

void foo(int i) 
{
  if (i == 1)
    return;
  std::map<int, int> map1; // constructor is executed here
  if (i == 2)
    return;
  std::map<int, int> map2; // constructor is executed here
}

For C, the story is different. It depends on architecture and compiler. For x86 and GCC, putting all the declarations at function beginning and declaring variables only when needed have the same performance. The reason is that C variables don't have a constructor. And the effect on stack memory allocation by these two approaches is the same. Here is an example:

void foo(int i)
{
  int m[50];
  int n[50];
  switch (i) {
    case 0:
      break;
    case 1:
      break;
    default:
      break;
  }
}

void bar(int i) 
{
  int m[50];
  switch (i) {
    case 0:
      break;
    case 1:
      break;
    default:
      break;
  }
  int n[50];
}

For both functions, the assembly code for stack manipulation is:

pushl   %ebp
movl    %esp, %ebp
subl    $400, %esp

Putting all the declarations at function beginning is common in Linux kernel code.

score 1 · Answer 7 · answered Sep 22 '10 at 20:43

Memory allocation is not specified in the Standard to this detail, so for a real answer you'll have to specify compiler and platform. It's not going to matter for performance.

What you want is readability, and in general that's done by declaring variables in the smallest usable scope, and preferably when you can immediately initialize them with reasonable values. The smaller a variable's scope, the less it can potentially interact with the rest of the program in unpredictable ways. The closer the declaration to initialization, the less opportunity for anything bad to happen.

What would probably be better is something like

RECT rc;
GetClientRect(hwnd, &rc);
PAINTSTRUCT ps;
HDC hdc = BeginPaint(hwnd, &ps);

This is for C++. For C, the rule is similar, except that earlier versions of C required all the variables to be declared at the top of a block.

score 1 · Answer 8 · answered Sep 22 '10 at 20:46

1

You can't know at what point the stack reservation is done.

For readability I would go with C99 (or C++). That allows you the declaration of a variable really there where first use it.

 HDC hdc = BeginPaint(hwnd, &ps);

answered Sep 22 '10 at 20:46

Jens Gustedt

76,821
6
102
177

score 0 · Answer 9 · answered Sep 22 '10 at 21:52

0

There's no need to pollute the stack with variables that are possibly never used. Allocate your vars right before they are used. Overlooking the RECT rc and the subsequent call to GetClientRect, Ben Voight's method is the way to go.

answered Sep 22 '10 at 21:52

myeviltacos

113
5

Declare variables at top of function or in separate scopes?

Method 1:

Method 2:

9 Answers9

Linked

Related