изменено предупреждение о закрытии в ReSharper - PullRequest
Новая
       26

изменено предупреждение о закрытии в ReSharper

4 голосов
/ 01 июня 2010

Я надеялся, что кто-нибудь сможет объяснить мне, что может случиться в этом коде с плохой вещью, в результате чего ReSharper выдает предупреждение «Доступ к измененному закрытию»: 

bool result = true;

foreach (string key in keys.TakeWhile(key => result))
{
    result = result && ContainsKey(key);
}

return result;

Даже если приведенный выше код кажется безопасным, что плохого может случиться в других случаях «модифицированного замыкания»? Я часто вижу это предупреждение в результате использования запросов LINQ, и я склонен игнорировать его, потому что я не знаю, что может пойти не так. ReSharper пытается решить проблему, создав вторую переменную, которая кажется мне бессмысленной, например, это меняет строку foreach выше на: 

bool result1 = result;
foreach (string key in keys.TakeWhile(key => result1))

Обновление: в примечании, очевидно, что весь кусок кода может быть преобразован в следующий оператор, который не вызывает предупреждений об изменении закрытия: 

return keys.Aggregate(
    true,
    (current, key) => current && ContainsKey(key)
);

Ответы [ 5 ]

8 голосов
/ 01 июня 2010

В этом конкретном коде ничего, возьмем в качестве примера следующее: 

int myVal = 2;

var results = myDatabase.Table.Where(record => record.Value == myVal);

myVal = 3;

foreach( var myResult in results )
{
    // TODO: stuff
}

Похоже, что результат вернет все записи, где значение равно 2, потому что это то, что было установлено myVal, когда вы объявили запрос. Однако из-за отложенного выполнения все записи будут иметь значение 3, поскольку запрос не будет выполнен, пока вы не выполните итерацию.

В вашем примере значение не изменяется, но Resharper предупреждает вас, что это может быть и что отложенное выполнение может вызвать проблемы.

6 голосов
/ 01 июня 2010

Когда вы изменяете переменную result, замыкание (использование переменной внутри лямбда-выражения) перехватит изменение.

Это часто бывает неожиданным сюрпризом для программистов, которые не совсем понимают замыкания, поэтому у Resharper есть предупреждение.

Создав отдельную переменную result1, которая используется только в лямбда-выражении, он будет игнорировать любые последующие изменения исходной переменной result.

В вашем коде вы полагаетесь на то, что замыкание забирает изменения в исходной переменной, чтобы она знала, когда остановиться.

Кстати, самый простой способ написать вашу функцию без LINQ - это: 

foreach (string key in keys) {
    if (ContainsKey(key))
        return true;   
}
return false;

Используя LINQ, вы можете просто позвонить Any(): 

return keys.Any<string>(ContainsKey);
3 голосов
/ 01 июня 2010

См.

http://ericlippert.com/2009/11/12/closing-over-the-loop-variable-considered-harmful-part-one/

для подробного обсуждения этой проблемы и того, что мы могли бы сделать в гипотетических будущих версиях C #, чтобы смягчить ее.

2 голосов
/ 01 июня 2010

Я не уверен, что ReSharper выдаст точно такое же предупреждение для этого, но следующее иллюстрирует похожую ситуацию.Итератор для цикла используется в предложении LINQ, но это предложение фактически не оценивается до тех пор, пока цикл не завершится, и к тому времени переменная итератора изменилась.Ниже приведен надуманный пример, который выглядит так, как будто он должен печатать все нечетные числа от 1 до 100, но фактически печатает все числа от 1 до 99. 

var notEven = Enumerable.Range(1,100);
foreach (int i in Enumerable.Range(1, 50))
{
    notEven = notEven.Where(s => s != i * 2);
}

Console.WriteLine(notEven.Count());
Console.WriteLine(string.Join(", ", notEven.Select(s => s.ToString()).ToArray()));

Это может быть довольно простой ошибкой.Я слышал, что люди говорят, что если вы действительно понимаете замыкания / функциональное программирование / и т.д.Вы никогда не должны делать эту ошибку.Я также видел, как профессиональные разработчики, которые, безусловно, хорошо разбираются в этих концепциях, совершают именно эту ошибку.

0 голосов
/ 01 июня 2010

Что ж, предупреждение связано с тем, что result может быть изменено до выполнения замыкания (переменные принимаются при исполнении, а не при определении). В вашем случае вы на самом деле используете этот факт. Если вы используете перекодировку с более резким выражением, это фактически нарушит ваш код, так как key => result1 всегда возвращает true.

...