Sunday, May 23, 2010

Anonymous methods / Lambda expression variable capture scope in C#

Let's face it, at some point or another, we've come across the problem in C# where you pass a for-loop variable into an anonymous method without redeclaring it and find that it doesn't behave expectedly. Once we get used to the idea of always redeclaring a variable you're going to use in your lambda expression though, you tend to ignore the fact that there is actually no reason why it should be the way it is.

At least, I find it hard to explain to a junior team member the reason behind this implementation where the compiler would not consider a for-loop variable as a local variable to its scope when it comes to anonymous methods. In case you're wondering, this is the reason why we need to redeclare 'var s' in the following example:

foreach (var s in Names)
{
var temp = s; // redeclaration

criteria = criteria.Or(x => x.Name == temp);
}

The above example is very obvious, no doubt. But, throw in some code refactoring (out to a separate method and merging inline again etc.) and more complex lambda expression, you'll find that it is quite easy to forget that 'temp' variable. A bad language is one that requires you to 'tip toe around a field of land mine' when you're using it - which defeats the whole selling point of C# (or at least what Eric Gunnerson was selling when he first introduced C# to the world).

Like I said, there's really no reason why the compiler shouldn't consider 'var s' as the local variable of the foreach block - there's no way you could access variable 's' outside of it anyway. In this case, variable 's' isn't local enough to be captured by the lambda expression, but isn't any more global than that either!

My suggestion to Microsoft is to have the compiler automatically insert the redeclaration (as a quick and simple fix). I really can't think of a genuine use-case where one would actually desire the behavior without the redeclaration. In fact, if that is truly what is intended, then wouldn't this make more sense and a LOT more readable (read: maintainable)?

var s = Names[Names.Length-1];

for (int i=0; i<Names.Length; i++)
{
criteria = criteria.Or(x => x.Name == s);
}

rather than,
foreach (var s in Names)
{
criteria = criteria.Or(x => x.Name == s);
}

No comments: