What am I missing in this chain of predicates?
.net, c#, delegates, linq, predicate
Solution
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:
- http://blogs.msdn.com/ericlippert/archive/2009/11/12/closing-over-the-loop-variable-considered-harmful.aspx
- http://blogs.msdn.com/ericlippert/archive/2009/11/16/closing-over-the-loop-variable-part-two.aspx
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:
- http://blogs.msdn.com/oldnewthing/archive/2006/08/02/686456.aspx
- http://blogs.msdn.com/oldnewthing/archive/2006/08/03/687529.aspx
- http://blogs.msdn.com/oldnewthing/archive/2006/08/04/688527.aspx
Problem
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?