Why do local variable?

Question

resharper offers me to make this a local variable, and writes "access to modified closure"

if (filter != null)
{
    if (filter.CityId != 0)
    {
        ads = ads.Where(x => x.Ad.CityId == filter.CityId);
    }
    if (filter.BusinesCategoryId != 0)
    {
        ads = ads.Where(x => x.BusinessCategoryId == filter.BusinesCategoryId);
    }
}

Why do local variable filter?

Which local variable , `ads`? – Tigran Apr 12 '12 at 05:32 — Tigran, Apr 12 '12 at 05:32

score 6 · Accepted Answer · edited May 23 '17 at 12:16

6

Because your query(Where(...)) is not being executed. I assume filter is obtained from a loop?

Linq query are not executed until they are used. so if you looped though a bunch of filters then started executing them later, the filter value would be wrong in the query.

A similar question : Access to Modified Closure Also: http://devnet.jetbrains.net/thread/273042

would need to see more code to 100% sure.

edited May 23 '17 at 12:16

Community

1
1

answered Apr 12 '12 at 05:32

TheRealTy

2,409
3
22
32

it is dangerous when there is a cycle? I do not cycle. – Mediator Apr 12 '12 at 06:38
2

Closures can be modified without a loop. The loop is the most common cause of unintentionally modified closure variables, but it's not the only cause. – phoog Apr 12 '12 at 06:43

score 3 · Answer 2 · edited May 23 '17 at 11:56

From how I understand this, Resharper will throw an error if you access a variable from a delegate (closure), and then modify the variable before you execute the delegate. This mostly happens if you access a for loop variable inside a delegate/lambda and execute it outside the loop. If your code is like:

foreach (filter in filters)
{
      if (filter != null)  {
            if (filter.CityId != 0)      {
                ads = ads.Where(x => x.Ad.CityId == filter.CityId);
            }
            if (filter.BusinesCategoryId != 0)      {
                ads = ads.Where(x => x.BusinessCategoryId == filter.BusinesCategoryId);
            }
      }
} 
return ads.ToList()

Then it will not behave like you'd expect it to. But if you execute the lambda expressions inside the loop scope then you would have no problem.

I wont explain why it behaves that way because a lot of people already explained it very well:

UPDATE: To answer "Why do local variable?" is because the fix to the above problem is using a local variable (i.e., inside the loop) and using that in your lambda. That way you are closing over the different instances of the variable for each instance of the lambda.

Why do local variable?

2 Answers2