Does this code really cause an "access to modified closure" problem?

Question

Taking the following code, Resharper tells me that voicesSoFar and voicesNeededMaximum cause "access to a modified closure". I read about these but what puzzles me here is that Resharper suggests fixing this by extracting the variables to right before the LINQ query. But that is where they are already!

Resharper stops complaining if I merely add int voicesSoFar1 = voicesSoFar right after int voicesSoFar = 0. Is there some weird logic I do not understand that makes Resharper's suggestion correct? Or is there a way to safely "access modified closures" in cases like these without causing bugs?

// this takes voters while we have less than 300 voices    
int voicesSoFar = 0;    
int voicesNeededMaximum = 300;    
var eligibleVoters =
    voters.TakeWhile((p => (voicesSoFar += p.Voices) < voicesNeededMaximum));

Answer 1

I suspect that modifying the 'voicesSoFar' value in TakeWhile is causing the problem.

Answer 2

You have a very nasty problem that arises from mutating an outer variable to the lambda expression. The problem is this: if you try to iterate eligibleVoters twice (foreach(var voter in eligibleVoters) { Console.WriteLine(voter.Name); } and immediately after (foreach(var voter in eligibleVoters) { Console.WriteLine(voter.Name); }) you will not see the same output. That is just not right from a functional programming perspective.

Here is an extension method that will accumulate until some condition on the accumulator is true:

public static IEnumerable<T> TakeWhileAccumulator<T, TAccumulate>(
    this IEnumerable<T> elements,
    TAccumulate seed,
    Func<TAccumulate, T, TAccumulate> accumulator,
    Func<TAccumulate, bool> predicate
) {
    TAccumulate accumulate = seed;
    foreach(T element in elements) {
        if(!predicate(accumulate)) {
            yield break;
        }
        accumulate = accumulator(accumulate, element);
        yield return element;
    }
}

Usage:

var eligibleVoters = voters.TakeWhileAccumulator(
                         0,
                         (votes, p) => votes + p.Voices, 
                         i => i < 300
                     );

Thus the above says accumulate voices while we have accumulated less than 300 votes.

Then with:

foreach (var item in eligibleVoters) { Console.WriteLine(item.Name); }
Console.WriteLine();
foreach (var item in eligibleVoters) { Console.WriteLine(item.Name); }

Output is:

Alice
Bob
Catherine

Alice
Bob
Catherine

Answer 3

Well, the error message is correct in as much that the value of voicesSoFar is not preserved during the operation. In pure "functional" terms (and lambdas are really designed to act functionally) it will be confusing.

For example, an interesting test would be:

what happens if I iterate the query twice?

For example:

int count = voters.Count();
var first = voters.FirstOrDefault();

I believe you can see... 10, null - confusing. The following would be repeatable:

public static IEnumerable<Foo> TakeVoices(
    this IEnumerable<Foo> voices, int needed)
{
    int count = 0;
    foreach (Foo voice in voices)
    {
        if (count >= needed) yield break;
        yield return voice;
        count += voice.Voices;
    }
}
....
foreach(var voice in sample.TakeVoices(numberNeeded)) {
    ...
}

If you needed you could of course write a reusable extension method that took a lambda.