0

I need to call the method for each item in the list. So, I have used Where<> query as follows,

List<string> list = new List<string>();
list.Add("Name1");
list.Add("Name2");
list.Add("Name3");

var name = list.Where(n =>
{
    return CheckName(n);
});

But in the above case, CheckName() is not hit. The same method is triggered if I use FirstOrDefault<>. I don't know whether it is a framework break or I am going in a wrong way.

As additional info, I am using.NET Framework 4.5.

Has anyone experienced this error? If so, is there any solution to overcome this issue?

SharpGobi
  • 144
  • 1
  • 13
Divakar
  • 514
  • 1
  • 9
  • 25

5 Answers5

3

You are understanding incorrectly the result of the Where condition. As linq is deffered executed it will only enter the where condition when materialized (by a ToList/FirstOrDefault/Sum and such).

The Where is never actually materialized in your current code (It did as you experienced when using FirstOrDefault) and as such it will never enter the CheckName method. Then, as Where will never return null but "worst case" an empty collection, which is not null, the result is true.

If you debug you will see that name equals true at the end of this. To "overcome" this depends on what is your desired output:

  1. If you want to know if you have any item that matched the predicate then:

    var result = list.Any(CheckName);
    
  2. If you want to retrieve those that match the predicate:

    var result = list.Where(CheckName);
    

    If later you want to query and check if results contains anything then:

    if(result.Any()) { /* ... */ }
    

    If you only want the results (and thus materializing the query):

    list.Where(CheckName).ToList();
    

Read more about linq being deffered executed here:


Just as a side note see how you can change your current code from:

var name = list.Where(n =>
{
    return CheckName(n);
})

To:

var name = list.Where(n => CheckName(n));

And eventually to:

var name = list.Where(CheckName);
Graham
  • 7,431
  • 18
  • 59
  • 84
Gilad Green
  • 36,708
  • 7
  • 61
  • 95
  • I have used the below code, var name = list.Where(n => CheckName(n)); But still the CheckName() method is not executed. – Divakar Jul 06 '17 at 06:30
  • @Divakar - yes - as I explained you must materialize the query. Using `ToList` for example – Gilad Green Jul 06 '17 at 06:31
  • It's poor code though, consider changing it so the next developer who works on it doesn't have to sit and think "what the heck is this block of code supposed to do" – Caius Jard Jul 06 '17 at 06:44
  • @CaiusJard what about it is poor code and what you'd suggest to improve? – Gilad Green Jul 06 '17 at 06:45
  • gilad, the comemnt was to divakar.. Why is it poor code? Well.. You've read what Divakar wrote, now tell me what it does! Kinda impossible, because the variable/method naming is poor, it doesn't self document. We can only say "it stores a collection of names in a variable called 'name' based on whether the return value of calling the CheckName method on the name returns true".. How about something like `var validNames = allNames.Where(IsLongerThan8Characters)` – Caius Jard Jul 06 '17 at 11:51
2

LINQ has a Deferred Execution principal which means the query will not be executed until and unless you access name variable. If you want to execute it immediately, (just for example) add .ToList() in the end, which is exactly what FirstOrDefault does. It does immediate execution instead of deferred execution.

var name = list.Where(n =>
{
    return CheckName(n);
}).ToList() != null;

Also where condition result will never be null. Even if there is no object in list satisfying your condition(s) in CheckName, where will return an empty collection.

Nikhil Agrawal
  • 47,018
  • 22
  • 121
  • 208
1

The CheckName() method is not executed because of Deferred execution of Linq. The actual statement is not executed till you actually access it. So in your case, for the CheckName(), you should do something like:

var name = list.Where(n =>
{
    return CheckName(n);
}).ToList();
Habeeb
  • 7,601
  • 1
  • 30
  • 33
0

If you need to call a method for each item in a list then you should use a simple for loop:

foreach var name in list
  CheckName(name);

Just because LINQ is available, doesn't mean it should be used everywhere there is a collection. It is important to write code that makes sense and is self commenting and using it here has simultaneously introduced a flaw into your logic and made your code harder to read, understand and maintain. It's the wrong tool for the stated purpose

Doubtless you have additional requirements not stated here, like "I want to check every name in a list and make sure that none are null". You can and possibly should use linq for this but it looks more like

bool allNamesOK = list.All(n => n != null);

This code is compact and reads well; we can clearly see the intention (though I wouldn't call the list "list" - "names" would better)

Caius Jard
  • 72,509
  • 5
  • 49
  • 80
0

When you look at the Where-Method source code you can easily see why:

    internal static IEnumerable<T> Where<T>(this IEnumerable<T> enumerable, Func<T, bool> where) {
        foreach (T t in enumerable) {
            if (where(t)) {
                yield return t;
            }
        }
    }

The yield will cause the execution to only happen once the returned IEnumerable<T> is actually accessed. That is what is called deferred execution.

Adwaenyth
  • 2,020
  • 12
  • 24