The C# "bug" that will catch you.

7    03 Apr 2015 09:36 by u/rwbj

First, some ostensibly simple code:

            int[] someData = new int[2];
            List<Action> someSetters = new List<Action>();
            for (int i = 0; i < someData.Length; i++)
                someSetters.Add(() => { someData[i / 2] = 2; });
            for (int i = 0; i < someData.Length; i++)
                someSetters[i]();

After this block of code finishes what are the values in someData? It seems pretty obvious. You create two Actions that should both do the same thing. Namely they should change the 0th index of the someData array to 2 so the array should contain {2,0}. What does it actually contain? {0,2}.

Why?

C# has a quirk that can lead to some nasty bugs. The thing is that in C# lambda expressions don't follow the normal scoping rules. You might already be able to see the problem from that sentence alone. Yip, when you write 

someSetters.Add(() => {someData[i/2] = 2;}

that lambda is actually grabbing a reference to i, and not the immediate value of i. By the time the loop

for (int i = 0; i < someData.Length; i++)

finishes the value of i is 2. So your Actions don't actually set the 0th index to 2, they set the 1st index to 2!!

The Fix

It's simple to fix. Here's the program functioning as intended:

        int[] someData = new int[2];
        List<Action> someSetters = new List<Action>();
        for (int i = 0; i < someData.Length; i++)
        {
            int temp = i;
            someSetters.Add(() => { someData[temp / 2] = 2; });
        }
        for (int i = 0; i < someData.Length; i++)
            someSetters[i]();

It may look somewhat ridiculous but the code now functions exactly as you'd normally expect. This is one nasty bug to keep in mind since it creeps up in locations you'd probably never bother to look for and is the only complete "wtf?" decision in the current C# standard.

12 comments

3 u/taxation_is_slavery 03 Apr 2015 14:30

I love C99. She ain't real pretty, but at least she isn't a cheating whore and does exactly what I tell her to.

1 u/Gamerdog6482 03 Apr 2015 23:35

C++ and C are just joys to use. C# is like the Javascript of C.

1 u/PuttItOut 03 Apr 2015 16:10

If you had this bug in JavaScript you'd be laughed and mocked out of a job but in C# it's a big issue?

0 u/rwbj [OP] 03 Apr 2015 20:40

Well you're comparing things that are very different. In javascript there's no such thing as block scoping so this behavior is somewhat expected. In C# everything is block scoped. For instance, the following code is actually a compile time error:

{
    {
        int a = 3;
    }
    SomeMethod(a);
}

As is the following:

{
    for( int i = 0; i < 10; i++)
        i++;
    SomeMethod(i);
}
0 u/PuttItOut 03 Apr 2015 20:44

JavaScript closures and C# lambda scoping are identical (nearly).

0 u/rwbj [OP] 03 Apr 2015 20:52

Exactly, and C# is not Javascript. It's not idiomatic. It provides 0 benefit. It's just a gotcha that should have been fixed long ago.

0 u/PuttItOut 03 Apr 2015 21:07

It's really not a gotcha or something that should be fixed IMO. It's how lambdas work, they are executed in the scope/context of their creation, they have to be. So a lambda's scope variables can always change.

To me this is not a bug, it's an education/testing thing. Devs should understand how lambda scoping works (what block is in scope when lambda is created) and then this isn't an issue. Lambda scoping is still block scoping at the root though, just which block is the confusing part I suppose.

1 u/rwbj [OP] 03 Apr 2015 22:18

Think about what you're saying. When you're arguing that something is okay because it's a matter of "education/testing" you're essentially engaging in circular logic along the lines of, "This works this way because it works this way." Many things in C# could have been simply written off as "education/testing" things but they're not. 

string s1 = "dog";
string s2 = "dog";
bool hmm = s1 == s2;

A very simple concept, but actually very pertinent. Strings need to be implemented as reference types for obvious reasons. However, the intent of that line of code is looking to compare pointers close to 0% of the time. Java stayed true to consistency and that statement would return false. C# took the path of intent and that statement returns true with a opaquely overloaded == operator.

Languages are tools. They're nothing more than hammers. Of course you do need to spend some time learning aspects of various languages, but the reasons for that learning should be immediately attributable to various benefits. When they're not, it's not a matter of teaching the blacksmith how to work around the tool's faults - it's a matter of improving the tool. The one reason I could see anybody defending this is that any fix would inherently break old code. And I suppose that's a fair point. A mistake that we can learn from when the standard for the next generation of tools is being developed.

1 u/PuttItOut 03 Apr 2015 23:41

It's not a fault, it's bad code.

Bad code is bad, just like your bad code examples that aren't braced properly.

I think Microsoft should force you to brace every for, foreach, if, if/else, etc statement because unbraced blocks can present bugs if the developer doesn't have the education to indent properly. Now argue that we don't need that safety in C# using your same logic.