Alex Rivera | Logout

What am I missing in this chain of predicates?

Asked 2010-03-05T19:57:27.407
10

NOTE: Right before posting this question it occurred to me there's a better way of doing what I was trying to accomplish (and I feel pretty stupid about it):

IEnumerable<string> checkedItems = ProductTypesList.CheckedItems.Cast<string>();
filter = p => checkedItems.Contains(p.ProductType);

So OK, yes, I already realize this. However, I'm posting the question anyway, because I still don't quite get why what I was (stupidly) trying to do wasn't working.

I thought this would be extremely easy. Turns out it is giving me quite a headache.

The basic idea: display all the items whose ProductType property value is checked in a CheckedListBox.

The implementation:

private Func<Product, bool> GetProductTypeFilter() {
    // if nothing is checked, display nothing
    Func<Product, bool> filter = p => false;

    foreach (string pt in ProductTypesList.CheckedItems.Cast<string>()) {
        Func<Product, bool> prevFilter = filter;
        filter = p => (prevFilter(p) || p.ProductType == pt);
    }

    return filter;
}

However, say the items "Equity" and "ETF" are both checked in ProductTypesList (a CheckedListBox). Then for some reason, the following code only returns products of type "ETF":

var filter = GetProductTypeFilter();
IEnumerable<Product> filteredProducts = allProducts.Where(filter);

I guessed it might have had something to do with some self-referencing messiness where filter is set to, essentially, itself or something else. And I thought that maybe using ...

filter = new Func<Product, bool>(p => (prevFilter(p) || p.ProductType == pt));

...would do the trick, but no such luck. Can anybody see what I am missing here?

c# .net linq delegates predicate
Edit
Report

1 Answer

9

I believe you have a modified closure problem here. The pt parameter is bound into the lambda expression but changes as the loop progresses. It's important to realize the when a variable is referenced in a lambda it is the variable that is captured, not the value of the variable.

In loops this has a very significant ramification - because the loop variable is changing, not being redefined. By creating a variable inside the loop, you are creating a new variable for each iteration - which then alows the lambda to capture each independently.

The desired implementation would be:

foreach (string pt in ProductTypesList.CheckedItems.Cast<string>()) {
    string ptCheck = pt;
    Func<Product, bool> prevFilter = filter;
    filter = p => (prevFilter(p) || p.ProductType == ptCheck);
}

Eric Lippert has written about this specific situation:

Also, see the question Access to Modified Closure (2) for a good explanation of what happens with closure variables. There's also an series of articles on the blog The Old New Thing that has an interesting perspective on this:

answered 2010-03-05T20:06:38.267

Your Answer