Category Archives: 17144

Lifting ForEach, Breaking Change in Visual Studio 2012

This post is updated due to the STUPID error I originally made in the last code sample. Thanks to the folks that pointed it out!


Also, I neglected to thank Adil Mughal for grabbing the ReSharper warnng text for me.


There’s a breaking change you should know about in Visual Studio 2012. In case you have a short attention span, aka have a life, I’ll give you the punch line first. The behavior of closures has changed in the specific instance of a lambda expression.


Good news: You aren’t writing the affected code on purpose, or if you are I honestly want to know why


Bad news: I don’t think anyone can guarantee their code doesn’t contain the issue


Good news: It’s a side case, the affected code probably isn’t there


Bad news: If it is in your program, its behavior will change when compiled with VS 2012


Good news: It probably won’t be too bad if the behavior changes


Bad news: The change symptom might be obtuse or bizarre


Good news: If you use VB or ReSharper there’s a warning and you just need to look for it


Bad news: If you’re in C# there’s no warning


Good news: You can download ReSharper’s 30 day free trial and run it against all of your code in 30 days (yes, I asked before posting this suggestion)


Observe the Change


Run this code in a console application in Visual Studio 2010. You might want to predict the result before continuing with the blog post:


 


class Program
{
    static void Main(string[] args)
    {
        ClosuresForEachWithLambda();
        Console.Read();
    }

    public static void ClosuresForEachWithLambda()
    {
        var actions = new List<Action>();
        var list = new int[] { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 };
        foreach (var i in list)
        { actions.Add(() => Console.WriteLine(i)); }

        foreach (var i in actions)
        { i(); }
    }


}

 


Or the identical code for Visual Basic


Option Strict On

Module Main

    Sub Main()
        ClosuresForEachWithLambda()
        Console.Read()
    End Sub

    Public Sub ClosuresForEachWithLambda()
        Dim actions = New List(Of Action)()
        Dim list = New Int16() {0, 1, 2, 3, 4, 5, 6, 7, 8, 9}
        For Each i In list
            actions.Add(Sub() Console.WriteLine(i))
        Next

        For Each i In actions
            i()
        Next
    End Sub

End Module

The Answer and the Explanation


While you might expect this code to print 0,1,2,3, etc, it actually prints 9,9,9,9, etc.


When variables are used in a lambda expression, the actual variable goes along for the ride. Not a copy, not the value but the actual variable. This is called a closure and is essential to you getting the behavior you expect at many other points in .NET. You can also find examples in this blog entry by Frans Bouma and a different example by Dmitri Nesteruk.


This is a significant enough issue that both Visual Basic and ReSharper issue a warning. C# without ReSharper does not issue a warningThe Visual Basic warning is

Warning

1

Using the iteration variable in a lambda expression may have unexpected results. Instead, create a local variable within the loop and assign it the value of the iteration variable.


The ReSharper warning is


Access to modified closure


The Visual Studio 2012 Change


When a lambda expression uses the looping variable of a for each loop in either Visual Basic or C#, the Visual Studio 2012 compiler creates a new variable scoped to the loop and assigns the value of the looping variable to this variable. Logically it’s the same as:


 


 


public static void ClosuresForEachWithLambda()
{
    var actions = new List<Action>();
    var list = new int[] { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 };
    foreach (var i in list)
    {
        // Changes are here
        var x = i;
        actions.Add(() => Console.WriteLine(x));
        // End changes
    }

    foreach (var i in actions)
    { i(); }
}

 


 


Since the new variable is scoped to the loop, there’s a new copy for every iteration of the loop. The original code compiled in Visual Studio 2012 prints 0,1,2,3… Try it!


Why this is a Really Good Thing to Change


I’ve been showing this code at user group presentations around the country and taken a few other opportunities to put this code in front of .NET developers – some of them really good .NET developers – and asking what the code would do.


The most common response is a few people in the room get it after about a minute. Sometimes, no one gets it. It’s exceedingly rare that anyone will have an immediate knee jerk response that it will print all nines. I get boatloads of “I should have seen that!” and very few “I saw it right away!” And, the very fact I am asking the question creates a context that the code probably doesn’t do what you’d expect and I made it as obvious as I possibly could. Your code is going to be a list of customers or invoices or something else. The vast majority of .NET developers do not expect the VS 2010 result, even though it is technically correct.


The async features of Visual Studio 2012 will create lambda expressions for you, and you will be much more likely to create lists of lambda expressions for simultaneous execution. Your ability to see this issue in your code gets significantly tougher in some VS 2012 scenarios. It was time to make this change.


It would have been nice if this had been an error from the beginning of lambda usage in .NET. But that didn’t happen and adding this as an error would have been a different kind of breaking change. For better or worse, the automatic fix won over adding an error or warning.


Is it a big deal?


I get asked a lot whether this is a big deal – sometimes even by people that are responsible for a code base.


It’s probably not a big deal in your code. The chances are excellent that you do not have code that uses a looping variable in a lambda expression. If you do, it’s probably a mistake – that’s why VB and ReSharper issue a warning. And it’s possible that any change in behavior will either be immensely obvious and perhaps trigger a test failure, or be so subtle that it never breaks your code at all.


But these are the wrong question to ask about a breaking change. The right question is “Can you guarantee that this does not exist anywhere in the code bases you are responsible for?” That’s the bar for a breaking change – can I guarantee I know how it affects me. The only way to do that is to run a tool that looks for the problem – check for the Visual Basic warning or run ReSharper.


I have had only one person say they were extremely confident that their unit tests would catch this change. I believe they are wrong. This is a side case that can give extremely unexpected behavior. It’s not a change I’d trust unit tests to find.


What’s Not Changed


This change happened because it can be very difficult to see the issue in a for each loop. It wasn’t done lightly and the teams decided to do the change in as narrow a scope as possible. That means the change was not made to for loops in either Visual Basic or C#. The following code returns 10,10,10, etc with both the VS 2012 and VS 2010 compilers.


 


public static void ClosuresForWithLambda()
{
    var x = new List<Action>();
    for (int i = 0; i < 10; i++)
    { x.Add(() => Console.WriteLine(i)); }

    foreach (var j in x)
    { j(); }
}

 


Or for VB


Public Sub ClosuresForWithLambda()
     Dim x = New List(Of Action)()
     For i = 0 To 9
         x.Add(Sub() Console.WriteLine(i))
     Next

     For Each j In x
         j()
     Next
 End Sub

The Critical Footnote


Any breaking compiler changes are a big deal. They aren’t such a big deal if we know about them, they are side cases and the new behavior is improved. The Visual Studio teams have announced that they are working to replace the compilers in a future version of Visual Studio. The current compilers are old and it seems highly unlikely that they can be rewritten without breaking changes. We can handle some breaking changes as long as the issues can be found with tools – specifically we can identify code that is clean of known behavior changes because of compiler changes. One of the reasons I want the community talking about this breaking change is that we need to establish expectations for all future breaking changes. So, talk about it. Run ReSharper. Tell your friends.!

INotifyPropertyChanged Implementation for VS2012

I told some folks at the East Tennessee .NET UG (ETNUG, which was adorably put on the reservation board at the after meeting bar as “Eatnug”) meeting in Knoxville that I would post info on a .NET 4.5 version of INotifyPropertyChanged that used CallerMemberName. I also said I wanted to reference other people rather than making up on my own because I’d rather post something already vetted.


CallerMemberName is a new feature of .NET 4.5 that lets you access the name of the thing that called the current method. It’s implemented by adding an attribute on an optional parameter:


public void Foo([CallerMemberName] string callerMemberName = null)


The CallerMemberName attribute signals that the caller parameter should contain the name of the calling method, to be filled in automatically if the calling code doesn’t pass a value. Since it’s an optional value, the calling code doesn’t need to pass a value, and should only pass one when chaining the “real” caller member name. Unfortunately, Intellisense doesn’t help you out here, so a really good parameter name, consistent across your project or organization is a great idea: I like callerMemberName as a direct indicator of the purpose/usage of the parameter.


There are a couple of obvious places to use CallerMemberName – logging and implementing INotifyPropertyChanged.


I asked on Twitter and checked around the Internet, and there are a bunch of simple examples that illustrate CallerMemberName. Here’s one in English, and one in Hungarian. There’s also a version in the documentation for the INotifyProertyChanged interface. If you’re interested in how CallerMemberName works, check these out.


If you’re interested in a bang-up, super-great, sea-salt and malt vinegar (potato chip) version, check out Dan Rigby’s blog posts here and especially here where he evaluates versions of INotifyPropertyChanged that have been used elsewhere. The basics are that the Set method should contain exactly one method call, the event should not be raised unless the property is actually changed to a new value, no strings should be used to avoid typo bugs, and no reflection or callstack access should be used to maintain performance.


I really like the flow of information on the Internet when people find something good and add to it. I have two enhancements I want to add – I hate having SetProperty in every class and I didn’t find a VB version.


First, you can use a common base class to avoid redundant SetProperty methods. This simplifies the data class. Note that it’s common to supply a protected OnPropertyChanged method in a base class. If this is called from something other than the property, such as the SetProperty method, the CallerMemberName can be passed in, passing on or chaining the original property name:


 


public class Foo : FooBase
 {
     private int _bar;
     public int Bar
     {
         get { return _bar; }
         set { SetProperty(ref _bar, value); }
     }
 }

 public abstract class FooBase : INotifyPropertyChanged
 {
     public event PropertyChangedEventHandler PropertyChanged;

     protected bool SetProperty<T>(
             ref T storage, T value,
         [CallerMemberName] String callerMemberName = null)
     {
         if (object.Equals(storage, value)) return false;

         storage = value;
         this.OnPropertyChanged(callerMemberName);
         return true;
     }

     protected void OnPropertyChanged(
             [CallerMemberName] string callerMemberName = null)
     {
         var eventHandler = this.PropertyChanged;
         if (eventHandler != null)
         {
             eventHandler(this,
                  new PropertyChangedEventArgs(callerMemberName));
         }
     }
 }


Since I didn’t find this available in VB, I translated:


Public Class Foo
    Inherits FooBase

    Private _bar As Integer
    Public Property Bar As Integer
        Get
            Return _bar
        End Get
        Set(value As Integer)
            SetProperty(_bar, value)
        End Set
    End Property

End Class

Public Class FooBase
    Implements INotifyPropertyChanged

    Public Event PropertyChanged(sender As Object, e As PropertyChangedEventArgs) _
            Implements INotifyPropertyChanged.PropertyChanged

    Protected Function SetProperty(Of T)(ByRef field As T, value As T,
                    <CallerMemberName> Optional callerMemberName As String = Nothing) _
                    As Boolean
        If Object.Equals(field, value) Then Return False

        OnPropertyChanged(callerMemberName)
        field = value
        Return True
    End Function

    Protected Sub OnPropertyChanged(
                    <CallerMemberName> Optional callerMemberName As String = Nothing)
        RaiseEvent PropertyChanged(Me, New PropertyChangedEventArgs(callerMemberName))
    End Sub

End Class