2009-11-06 15 views
34

tengo el siguiente código:ReSharper Advertencia - El acceso a Cierre Modificado

string acctStatus = account.AccountStatus.ToString(); 
if (!SettableStatuses().Any(status => status == acctStatus)) 
    acctStatus = ACCOUNTSTATUS.Pending.ToString(); 

Tenga en cuenta que account.AccountStatus es una enumeración de tipo accountstatus. En la segunda línea, ReSharper me está dando la advertencia "Acceso al cierre modificado" para acctStatus. Cuando hago la operación recomendada, Copiar a la variable local, modifica el código por lo siguiente:

string acctStatus = realAccount.AccountStatus.ToString(); 
string s = acctStatus; 
if (!SettableStatuses().Any(status => status == s)) 
    acctStatus = ACCOUNTSTATUS.Pending.ToString(); 

¿Por qué es mejor o preferible a lo que tenía originalmente?

EDITAR

También recomienda Wrap variable local en conjunto, que produce:

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

Esto parece francamente loco a mí.

+0

Verifique esta SO La pregunta y la respuesta aceptada, pueden ser útiles. http://stackoverflow.com/questions/235455/access-to-modified-closure – Chuck

Respuesta

34

El motivo de la advertencia es que dentro de un bucle puede estar accediendo a una variable que está cambiando. Sin embargo, la "solución" no está realmente haciendo nada por ti en este contexto sin bucle.

Imagine que tiene un bucle FOR y el si estaba dentro y la declaración de cadena estaba fuera de él. En ese caso, el error sería identificar correctamente el problema de obtener una referencia a algo inestable.

Un ejemplo de lo que no quiere:

string acctStatus 

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

El problema es que el cierre se agarra una referencia a acctStatus, pero cada iteración del bucle va a cambiar ese valor. En que caso sería mejor:

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

Como contexto de la variable es el bucle, una nueva instancia se creará cada vez porque hemos movido la variable dentro del contexto local (el bucle).

La recomendación suena como un error en el análisis de Resharper de ese código. Sin embargo, en muchos casos esto es una preocupación válida (como el primer ejemplo, donde la referencia está cambiando a pesar de su captura en un cierre).

Mi regla de oro es, en caso de duda hacer un local.

Aquí está un ejemplo del mundo real Me picaron por:

 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); 
     } 

Tenga en cuenta que capturo el tipo NodeType locales, observo nodeType y HistoryItem crumb.ItemGuid, no migajas [i] .ItemGuid. Esto asegura que mi cierre no tendrá referencias a elementos que cambiarán.

Antes de utilizar los locales, los eventos se activan con los valores actuales, no con los valores capturados que esperaba.

+0

Tiene mucho sentido, ¡gracias! –

+1

"El problema es que el cierre tomará una referencia a acctStatus, pero cada iteración de bucle cambiará ese valor" - en realidad siempre será 'account.AccountStatus [...]. ToString()' siempre que el predicado 'Any' lo evalúa (ya que 'Any' itera sobre el enumerable antes de regresar), así que no estoy seguro de que sea un buen ejemplo. Creo que lo que usted propuso como mejor tendrá el mismo comportamiento. –

+1

Esto sería incorrecto. El motivo de la diferencia es que en el primer ejemplo solo se asigna una variable: acctStatus. Resharper está preocupado de que esta variable cambie antes de que Any() se evalúe realmente. Mi sugerencia no cambia en absoluto la acción del código (Any() se evalúa antes de que ocurra ningún cambio) pero hace explícito que queremos una instancia distinta de la variable acctStatus para cada iteración del ciclo. Si observas mi ejemplo posterior, el error Resharper está preocupado por disparadores realmente porque * no * evalúo inmediatamente el lambda (menos los lugareños). – Godeke

Cuestiones relacionadas