ReSharper Warning – Access to Modified Closure

The reason for the warning is that inside a loop you might be accessing a variable that is changing. However, the “fix” isn’t really doing anything for you in this non-loop context.

Imagine that you had a FOR loop and the if was inside it and the string declaration was outside it. In that case the error would be correctly identifying the problem of grabbing a reference to something unstable.

An example of what you don’t want:

string acctStatus

foreach(...)
{
  acctStatus = account.AccountStatus[...].ToString();
  if (!SettableStatuses().Any(status => status == acctStatus))
      acctStatus = ACCOUNTSTATUS.Pending.ToString();
}

The problem is that the closure will grab a reference to acctStatus, but each loop iteration will change that value. In that case it would be better:

foreach(...)
{
  string acctStatus = account.AccountStatus[...].ToString();
  if (!SettableStatuses().Any(status => status == acctStatus))
      acctStatus = ACCOUNTSTATUS.Pending.ToString();
}

As the variable’s context is the loop, a new instance will be created each time because we have moved the variable inside the local context (the for loop).

The recommendation sounds like a bug in Resharper’s parsing of that code. However, in many cases this is a valid concern (such as the first example, where the reference is changing despite its capture in a closure).

My rule of thumb is, when in doubt make a local.

Here is a real world example I was bitten by:

        menu.MenuItems.Clear();
        HistoryItem[] crumbs = policyTree.Crumbs.GetCrumbs(nodeType);

        for (int i = crumbs.Length - 1; i > -1; i--) //Run through items backwards.
        {
            HistoryItem crumb = crumbs[i];
            NodeType type = nodeType; //Local to capture type.
            MenuItem menuItem = new MenuItem(crumb.MenuText);
            menuItem.Click += (s, e) => NavigateToRecord(crumb.ItemGuid, type);
            menu.MenuItems.Add(menuItem);
        }

Note that I capture the NodeType type local, note nodeType, and HistoryItem crumb.ItemGuid, not crumbs[i].ItemGuid. This ensures that my closure will not have references to items that will change.

Prior to using the locals, the events would trigger with the current values, not the captured values I expected.