Will the condition on a LINQ Where-statement re-evaluate during the loop?

Question

I have this foreach-loop:

foreach (var classId in m_ClassMappings[taAddressDefinition.Key])
{
    if(!m_TAAddressDefinitions.ContainsKey(classId))
    {
        m_TAAddressDefinitions.Add(classId, taAddressDefinition.Value);
    }
}

m_TAAddressDefinitions is an IDictionary where classId is used as the unique Key value.
Resharper suggests to change the if-statement into into a LINQ filter like this:

foreach (var classId in m_ClassMappings[taAddressDefinition.Key].Where(
    classId => !m_TAAddressDefinitions.ContainsKey(classId)))
{
    m_TAAddressDefinitions.Add(classId, taAddressDefinition.Value);
}

I am conserned if this might not work as expected, since the content of the m_TAAddressDefinitions collection is changing inside the loop, which causes the source of the LINQ filter-condition (classId => !m_TAAddressDefinitions.ContainsKey(classId)) to change on the way.

Will this fail if two classId with same value is added inside the loop, or will the LINQ condition be recalculated when values are added to the collection? My original if-statement was intended to not cause exception if the key already exist.

Sometimes, briefer code does not make for more readable code. The Resharper version is exactly the same as your old version, but as you're finding out, it's harder for you to understand. I'd recommend in that case to skip that refactor. — Will, Oct 14 '11 at 09:09

Steve Rowbotham · Accepted Answer · 2011-10-14T10:10:35.170

In this case, the fact that the IEnumerable returned by Where in the refactored version will lazily produce values as it is iterated over achieves what you want. The suggestion that includes the ToList call is not what you want in this case since that would materialize the IEnumerable as a collection and you would be vulnerable to there being duplicate classIds in the m_ClassMappings collection.

The thing to remember is that the predicate in the Where call, namely classId => !m_TAAddressDefinitions.ContainsKey(classId) will be evaluated for each item as it is produced as a result of iterating over the IEnumerable. So in the version suggested by Resharper, if there were duplicate values in the m_ClassMappings the first one encountered would be added to the m_TAAddressDefinitions but when the next duplicate is reached the Where predicate will return false because the value has been previously added to the Dictionary.

Great answer. This made me understand why it works. No matter, I will still use my original version, and not the LINQ - not because I don't trust it to work, but it is clearer what it does. — awe, Oct 14 '11 at 10:17

score 2 · Answer 2 · edited May 23 '17 at 12:03

2

The suggested change does not alter the logic. The lambda expression classId => !m_TAAddressDefinitions.ContainsKey(classId) will be compiled into a anonymous class where your dictionary is passed in. Since this is a reference and not a copy of it changes to the dictionary will be reflected in the evaluation.

This concept is known as closure. See this answer from Jon Skeet for more in depth information: What are 'closures' in .NET?. And his post The Beauty of Closures is a very code read.

edited May 23 '17 at 12:03

Community

1
1

answered Oct 14 '11 at 09:03

Stefan

14,530
4
55
62

But I would think the lambda is only called once (even if it's a dynamic reference) because the Where statement is just used to filter a collection, and return a result, which is used as collection on which the for loop is executed. – awe Oct 14 '11 at 09:48
No, since LINQ queries use [deferred execution](http://blogs.msdn.com/b/ericwhite/archive/2008/04/22/deferred-execution.aspx). – Stefan Oct 14 '11 at 10:02
OK. After reading the [answer by Steve Rowbotham](http://stackoverflow.com/questions/7765209/will-the-condition-on-a-linq-where-statement-re-evaluete-during-the-loop/7766070#7766070), I understand that your answer is also correct, so I give you +1. – awe Oct 14 '11 at 10:19

score 0 · Answer 3 · answered Oct 14 '11 at 08:56

0

You're not modifying m_ClassMappings but m_TAAddressDefinitions, so the CollectionModifiedException will not be raised.

Better rewrite it to

m_ClassMappings[taAddressDefinition.Key]
  .Where(classId => !m_TAAddressDefinitions.ContainsKey(classId)))  
  .ToList()
  .ForEach(
    classId => m_TAAddressDefinitions.Add(classId, taAddressDefinition.Value));

answered Oct 14 '11 at 08:56

Karel Frajták

4,389
1
23
34

I'm not conserned about modified exception on `m_ClassMappings`, but a "key already exists" exception when adding to `m_TAAddressDefinitions` if a classId already have been added previously in the same loop. I have very little experience with LINQ. Is filtered once, and in my case doing a loop on the result, or is the filter re-executed in each loop iteration? – awe Oct 14 '11 at 09:05
In LINQ the lamdba inside `Where` (and any method) is evaluated for each loop - so after calling `ToList` you will have list of keys not in the dictionary. You're solution is correct. – Karel Frajták Oct 14 '11 at 09:27
But the list will not be re-populated once inside the loop, so if there are multiple equal classId's within `m_ClassMappings[taAddressDefinition.Key]`, the call to Add will raise exception the second time because none of the classId's were in `m_TAAddressDefinitions` to begin with, but during the loop, the condition change. – awe Oct 14 '11 at 09:43
1

Materializing the `IEnumerable` with `ToList` isn't what you want. See my answer. – Steve Rowbotham Oct 14 '11 at 10:06

Will the condition on a LINQ Where-statement re-evaluate during the loop?

3 Answers3