модифицирано предупреждение за затваряне в ReSharper

Надявах се някой да може да ми обясни какво лошо нещо може да се случи в този код, което кара 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)
);

.net c# closures resharper
person Sarah Vessels    schedule 01.06.2010    source източник
comment
Можете да съкратите кода си до това result &= ContainsKey(key);, ако желаете.   -  person ChaosPandion    schedule 01.06.2010
comment
@ChaosPandion: това не е ли побитов оператор? ...Или това няма значение за bool?   -  person Sarah Vessels    schedule 01.06.2010
comment
ChaosPandion означава result &&= ContainsKey(key);   -  person SLaks    schedule 01.06.2010
comment
Дубликат на това и няколко други.   -  person adrianbanks    schedule 01.06.2010
comment
@adrianbanks: Вероятно трябваше да търся „модифицирано затваряне“; Просто не видях никакви идентични въпроси, когато въведох заглавието на моя въпрос във формуляра за изпращане.   -  person Sarah Vessels    schedule 01.06.2010

Отговори (5)


arrow_upward
6
arrow_downward

Когато модифицирате променливата result, затварянето (използването на променливата вътре в ламбда израза) ще вземе промяната.

Това често идва като неочаквана изненада за програмисти, които не разбират напълно затварянията, така че Resharper има предупреждение за това.

Чрез създаването на отделна result1 променлива, която се използва само в ламбда израза, тя ще игнорира всички последващи промени в оригиналната result променлива.

Във вашия код вие разчитате на затварянето да вземе промените в оригиналната променлива, така че да знае кога да спре.

Между другото, най-простият начин да напишете вашата функция без LINQ е така:

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

Използвайки LINQ, можете просто да извикате Any():

return keys.Any<string>(ContainsKey);
person SLaks    schedule 01.06.2010
comment
Така че редът, в който давам на result различна стойност, е това, което причинява предупреждението, тъй като result се използва и в ламбда в TakeWhile? - person Sarah Vessels; 01.06.2010
comment
Мисля, че вашата foreach замяна е грешна: методът трябва да върне true, ако всички keys са в речника (както е решено индивидуално от ContainsKey). Въпреки това return keys.All(ContainsKey); работи! - person Sarah Vessels; 01.06.2010

arrow_upward
8
arrow_downward

В този конкретен код няма нищо, вземете следното като пример:

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 ви предупреждава, че може да е и че отложеното изпълнение може да ви причини проблеми.

person Dave D    schedule 01.06.2010

arrow_upward
3
arrow_downward

Вижте

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

за обширно обсъждане на този проблем и какво можем да направим в хипотетични бъдещи версии на C#, за да го смекчим.

person Eric Lippert    schedule 01.06.2010

arrow_upward
2
arrow_downward

Не съм сигурен дали 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()));

Това може да бъде доста лесна грешка. Чувал съм хора да казват, че ако наистина разбирате затваряния/функционално програмиране/и т.н. никога не трябва да правите тази грешка. Виждал съм също така професионални разработчици, които със сигурност разбират добре тези концепции, да правят точно тази грешка.

person Daniel Plaisted    schedule 01.06.2010

arrow_upward
0
arrow_downward

Е, предупреждението е, защото result може да бъде променено преди затварянето да бъде изпълнено (променливите се вземат при изпълнение, а не дефиниране). Във вашия случай вие всъщност се възползвате от този факт. Ако използвате resharper reccomodation, това всъщност ще наруши вашия код, тъй като key => result1 винаги връща true.

person Femaref    schedule 01.06.2010