Declaring variables within FOR loops

Question

A weird bug was occurring in production which I was asked to look into.
The issue was tracked down to a couple of variables being declared within a For loop and not being initialized on each iteration. An assumption had been made that due to the scope of their declaration they would be "reset" on each iteration.
Could someone explain why they would not be)?
(My first question, really looking forward to the responses.)
The example below is obviously not the code in question but reflects the scenario:
Please excuse the code example, it looks fine in the editor preview??

for (int i =0; i< 10; i++)
{
    decimal? testDecimal;
    string testString;

    switch( i % 2  )
    {
        case 0:
        testDecimal = i / ( decimal ).32;
        testString = i.ToString();
            break;
        default:
            testDecimal = null;
            testString = null;
            break;
    }

    Console.WriteLine( "Loop {0}: testDecimal={1} - testString={2}", i, testDecimal , testString );
}

---

EDIT:

Sorry, had to rush out for child care issue. The issue was that the prod code had was that the switch statement was huge and in some "case"'s a check on a class' property was being made, like if (myObject.Prop != null) then testString = myObject.Stringval... At the end of the switch, (outside) a check on testString == null was being made but it was holding the value from the last iteration,hence not being null as the coder assumed with the variable being declared within the loop.
Sorry if my question and example was a bit off, I got the phone call about the day care as I was banging it together. I should have mentioned I compared IL from both variables in and out the loop. So, is the common opinion that "obviously the variables would not be reinitialized on each loop"?
A little more info, the variables WHERE being initialized on each iteration until someone got over enthusiastic with ReSharper pointing out "the value is never used" and removed them.

---

EDIT:

Folks, I thank you all. As my first post I see how much clearer I should be in the future. The cause of our unexpected variable assignment can me placed on an inexperienced developer doing everything ReSharper told him and not running any unit tests after he ran a "Code Cleanup" on an entire solution. Looking at the history of this module in VSS I see variables Where declared outside of the loop and where initialized on each iteration. The person in question wanted his ReSharper to show "all green" so "moved his variables closer to assignment" then "Removed redundant assignment"! I don't think he will be doing it again...now to spend the weekend running all the unit tests he missed!
How to do mark a question as answered?

Answer 1

Were you getting a NullReferenceException error?

From the code above you would get that error on every odd numbered iteration of the loop as you are trying to print the variables after you have assigned them as null.

Answer 2

EDIT: After Grants posting, it appears that what I was talking about (declaring the variables outside of the loop) is what the compiler is doing ANYWAY.

Neither way sucks up additional resources, because they both compile to the SAME thing (declaring outside of the loop).

Yes, readability of code is a good reason to declare in tight scope, BUT I wasn't arguing that and its not like the declarations were happening 50 lines away from the loop.

That being said, my post was valid in that it provided a solution to the posters problem with the added benefit of additional explination.

Kindly remove the down vote if you gave me one for this reason.

---

original:

You shouldn't put the declarations inside the for loop anyway. It sucks up additional resources for creating a variable over and over again, when what you should do is just clear the variable with each iteration.

I would agree with the previous poster that you are asking for NullRefEx by using nullable types in the String.format. See this construction instead.

decimal? testDecimal;
string testString;

for (int i =0; i< 10; i++)
{
    switch( i % 2  )
    {
        case 0:
           testDecimal = i / ( decimal ).32;
           testString = i.ToString();
           Console.WriteLine( "Loop {0}: testDecimal={1} - testString={2}", i, testDecimal , testString );
            testDecimal = null;
            testString = String.Empty;
            break;
        default:
            testDecimal = null;
            testString = String.Empty;
            break;
    }

}

EDIT: removed reflectors improperly disassembled code.

Answer 3

Here is the output of your code:

Loop 0: testDecimal=0 - testString=0
Loop 1: testDecimal= - testString=
Loop 2: testDecimal=6.25 - testString=2
Loop 3: testDecimal= - testString=
Loop 4: testDecimal=12.5 - testString=4
Loop 5: testDecimal= - testString=
Loop 6: testDecimal=18.75 - testString=6
Loop 7: testDecimal= - testString=
Loop 8: testDecimal=25 - testString=8
Loop 9: testDecimal= - testString=

I didn't change anything in your posted source to generate this output. Note it isn't throwing an exception either.

Answer 4

You shouldn't put the declarations inside the for loop anyway. It sucks up additional resources for creating a variable over and over again, when what you should do is just clear the variable with each iteration.

No, it does not! The exact opposite of your advice should be done. But even if it were more efficient to reset the variable, it's much clearer to declare the variable in its tightest possible scope. And clarity wins over micro-optimization (nearly) any time. Furthermore, one variable, one usage. Don't reuse variables unnecessarily.

That said, variables are not reset nor re-initialized here – actually, they are not even intialized by C#! To fix this, just initialize them and be done.

Answer 5

This does surprised me as well. I would have thought that scope would have changed inside a “for” loop. This does not seem to be the case. The values are being retained. The compiler seems to be smart enough to declare the variable one time when the “for” loop is first entered.

I do agree with the previous posts that you shouldn’t put the declarations inside of the “for” loop anyway. If you initialize the variable you will be sucking up resources on every loop.

But if you break the inner part of the “for” loop to a function (I know this is still bad). You go out of scope and the variables are created every time.

private void LoopTest()
{
    for (int i =0; i< 10; i++)
    {
        DoWork(i);
    }
}

private void Work(int i)
{
    decimal? testDecimal;
    string testString;

    switch (i % 2)
    {
        case 0:
            testDecimal = i / (decimal).32;
            testString = i.ToString();
            break;
        default:
            testDecimal = null;
            testString = null;
            break;
    }
    Console.WriteLine( "Loop {0}: testDecimal={1} - testString={2}", i, testDecimal , testString );
}

Well at least I learned something new. As well as just how bad declaring variable inside loops really are.

Answer 6

Here is the original source, supposedly using "more resources" because the variables are declared inside the loop:

using System;

class A
{
    public static void Main()
    {
        for (int i =0; i< 10; i++)
        {
            decimal? testDecimal;
            string testString;
            switch( i % 2  )
            {
                case 0:
                    testDecimal = i / ( decimal ).32;
                    testString = i.ToString();
                    break;
                default:
                    testDecimal = null;
                    testString = null;
                    break;
            }

            Console.WriteLine( "Loop {0}: testDecimal={1} - testString={2}", i, testDecimal , testString );
        }
    }
}

Here is the IL from the inefficient declaration source:

.method public hidebysig static void Main() cil managed
{
    .entrypoint
    .maxstack 8
    .locals init (
        [0] int32 num,
        [1] valuetype [mscorlib]System.Nullable`1<valuetype [mscorlib]System.Decimal> nullable,
        [2] string str,
        [3] int32 num2,
        [4] bool flag)
    L_0000: nop 
    L_0001: ldc.i4.0 
    L_0002: stloc.0 
    L_0003: br.s L_0061
    L_0005: nop 
    L_0006: ldloc.0 
    L_0007: ldc.i4.2 
    L_0008: rem 
    L_0009: stloc.3 
    L_000a: ldloc.3 
    L_000b: ldc.i4.0 
    L_000c: beq.s L_0010
    L_000e: br.s L_0038
    L_0010: ldloca.s nullable
    L_0012: ldloc.0 
    L_0013: call valuetype [mscorlib]System.Decimal [mscorlib]System.Decimal::op_Implicit(int32)
    L_0018: ldc.i4.s 0x20
    L_001a: ldc.i4.0 
    L_001b: ldc.i4.0 
    L_001c: ldc.i4.0 
    L_001d: ldc.i4.2 
    L_001e: newobj instance void [mscorlib]System.Decimal::.ctor(int32, int32, int32, bool, uint8)
    L_0023: call valuetype [mscorlib]System.Decimal [mscorlib]System.Decimal::op_Division(valuetype [mscorlib]System.Decimal, valuetype [mscorlib]System.Decimal)
    L_0028: call instance void [mscorlib]System.Nullable`1<valuetype [mscorlib]System.Decimal>::.ctor(!0)
    L_002d: nop 
    L_002e: ldloca.s num
    L_0030: call instance string [mscorlib]System.Int32::ToString()
    L_0035: stloc.2 
    L_0036: br.s L_0044
    L_0038: ldloca.s nullable
    L_003a: initobj [mscorlib]System.Nullable`1<valuetype [mscorlib]System.Decimal>
    L_0040: ldnull 
    L_0041: stloc.2 
    L_0042: br.s L_0044
    L_0044: ldstr "Loop {0}: testDecimal={1} - testString={2}"
    L_0049: ldloc.0 
    L_004a: box int32
    L_004f: ldloc.1 
    L_0050: box [mscorlib]System.Nullable`1<valuetype [mscorlib]System.Decimal>
    L_0055: ldloc.2 
    L_0056: call void [mscorlib]System.Console::WriteLine(string, object, object, object)
    L_005b: nop 
    L_005c: nop 
    L_005d: ldloc.0 
    L_005e: ldc.i4.1 
    L_005f: add 
    L_0060: stloc.0 
    L_0061: ldloc.0 
    L_0062: ldc.i4.s 10
    L_0064: clt 
    L_0066: stloc.s flag
    L_0068: ldloc.s flag
    L_006a: brtrue.s L_0005
    L_006c: ret 
}

Here is the source declaring the variables outside the loop:

using System;

class A
{
    public static void Main()
    {
        decimal? testDecimal;
        string testString;

        for (int i =0; i< 10; i++)
        {
            switch( i % 2  )
            {
                case 0:
                    testDecimal = i / ( decimal ).32;
                    testString = i.ToString();
                    break;
                default:
                    testDecimal = null;
                    testString = null;
                    break;
            }

            Console.WriteLine( "Loop {0}: testDecimal={1} - testString={2}", i, testDecimal , testString );
        }
    }
}

Here is the IL declaring the variables outside the loop:

.method public hidebysig static void Main() cil managed
{
    .entrypoint
    .maxstack 8
    .locals init (
        [0] valuetype [mscorlib]System.Nullable`1<valuetype [mscorlib]System.Decimal> nullable,
        [1] string str,
        [2] int32 num,
        [3] int32 num2,
        [4] bool flag)
    L_0000: nop 
    L_0001: ldc.i4.0 
    L_0002: stloc.2 
    L_0003: br.s L_0061
    L_0005: nop 
    L_0006: ldloc.2 
    L_0007: ldc.i4.2 
    L_0008: rem 
    L_0009: stloc.3 
    L_000a: ldloc.3 
    L_000b: ldc.i4.0 
    L_000c: beq.s L_0010
    L_000e: br.s L_0038
    L_0010: ldloca.s nullable
    L_0012: ldloc.2 
    L_0013: call valuetype [mscorlib]System.Decimal [mscorlib]System.Decimal::op_Implicit(int32)
    L_0018: ldc.i4.s 0x20
    L_001a: ldc.i4.0 
    L_001b: ldc.i4.0 
    L_001c: ldc.i4.0 
    L_001d: ldc.i4.2 
    L_001e: newobj instance void [mscorlib]System.Decimal::.ctor(int32, int32, int32, bool, uint8)
    L_0023: call valuetype [mscorlib]System.Decimal [mscorlib]System.Decimal::op_Division(valuetype [mscorlib]System.Decimal, valuetype [mscorlib]System.Decimal)
    L_0028: call instance void [mscorlib]System.Nullable`1<valuetype [mscorlib]System.Decimal>::.ctor(!0)
    L_002d: nop 
    L_002e: ldloca.s num
    L_0030: call instance string [mscorlib]System.Int32::ToString()
    L_0035: stloc.1 
    L_0036: br.s L_0044
    L_0038: ldloca.s nullable
    L_003a: initobj [mscorlib]System.Nullable`1<valuetype [mscorlib]System.Decimal>
    L_0040: ldnull 
    L_0041: stloc.1 
    L_0042: br.s L_0044
    L_0044: ldstr "Loop {0}: testDecimal={1} - testString={2}"
    L_0049: ldloc.2 
    L_004a: box int32
    L_004f: ldloc.0 
    L_0050: box [mscorlib]System.Nullable`1<valuetype [mscorlib]System.Decimal>
    L_0055: ldloc.1 
    L_0056: call void [mscorlib]System.Console::WriteLine(string, object, object, object)
    L_005b: nop 
    L_005c: nop 
    L_005d: ldloc.2 
    L_005e: ldc.i4.1 
    L_005f: add 
    L_0060: stloc.2 
    L_0061: ldloc.2 
    L_0062: ldc.i4.s 10
    L_0064: clt 
    L_0066: stloc.s flag
    L_0068: ldloc.s flag
    L_006a: brtrue.s L_0005
    L_006c: ret 
}

I'll share the secret, with the exception of the order in which .locals init ( ... ) are specified, the IL is exactly the same. DECLARING variables inside a loop results in NO ADDITIONAL IL.

Answer 7

something wierd is going on here, if they are never initialized, then it should throw a compile error.

when I ran your code, I got exactly what I would expect, nothing on the odd loops and the right numbers on the even loops.

Answer 8

Most of the time, it does not matter whether you declare a variable inside or outside the loop; the rules of definite assignment ensure that it doesn't matter. In the debugger you might occasionally see old values (i.e. if you look at a variable in a breakpoint before it is assigned), but static-analysis proves that this won't impact executing code. The variables are never reset per loop, as there is demonstrably no need.

At the IL level, *usually the variable is declared just once for the method - the placement inside the loop is just a convenience for us programmers.

HOWEVER there is an important exception; any time a variable is captured, the scoping rules get more complex. For example (2 secs):

        int value;
        for (int i = 0; i < 5; i++)
        {
            value = i;
            ThreadPool.QueueUserWorkItem(delegate { Console.WriteLine(value); });
        }
        Console.ReadLine();

Is very different to:

        for (int i = 0; i < 5; i++)
        {
            int value = i;
            ThreadPool.QueueUserWorkItem(delegate { Console.WriteLine(value); });
        }
        Console.ReadLine();

As the "value" in the second example is truly per instance, since it is captured. This means that the first example might show (for example) "4 4 4 4 4", where-as the second example will show 0-5 (in any order) - i.e. "1 2 5 3 4".

So: were captures involved in the original code? Anything with a lambda, an anonymous method, or a LINQ query would qualify.