12
\$\begingroup\$

I have two lists of objects Person and PersonResult. Both are linked through the property PersonId. I need to create a filter for the list of PersonResult that meet certain criteria for the Person (e.g. Person.Gender == "female").

Im currently using the following LINQ query to achieve this:

PersonResultList = PersonResultList.Where(pr => 
    PersonList.FirstOrDefault(p => pr.PersonId == p.PersonId) != null && 
    PersonList.FirstOrDefault(p => pr.PersonId == p.PersonId).Gender == "female");

This works apparently well, however, I must iterate twice through PersonList to check if the person exist and its gender. Is there a more elegant way to achieve this?

\$\endgroup\$
12
\$\begingroup\$

You can simply combine the conditions inside the FirstOrDefault() like

PersonResultList = PersonResultList
    .Where(pr => PersonList
                    .FirstOrDefault(p => pr.PersonId == p.PersonId 
                                         && p.Gender == "female") != null );  

Because I only changed your existing code, it didn't came to my mind what Nikita Brizhak commented here .

You should probably use Any instead of FirstOrDefault

So let us change the code to

PersonResultList = PersonResultList
        .Where(pr => PersonList
                        .Any(p => pr.PersonId == p.PersonId 
                              && p.Gender == "female")); 

This is based on the assumption that for each entry in the first list there will be only one entry in the second list.

\$\endgroup\$
  • 7
    \$\begingroup\$ You should probably use Any instead of FirstOrDefault :) \$\endgroup\$ – Nikita B Jun 18 '15 at 12:27
  • \$\begingroup\$ Does Any() replace the needs to check !=null I'm not that familiar with linq queries yet. \$\endgroup\$ – WizLiz Jun 18 '15 at 12:49
  • \$\begingroup\$ Any() checks if there is any item which meets the condition. If there is no item then this pr will not be included in the result. \$\endgroup\$ – Heslacher Jun 18 '15 at 12:51
  • 1
    \$\begingroup\$ The point of LINQ is not really speed, but ease of coding and maintainability. If a query is speed-critical, then you may have to hand-optimize it. \$\endgroup\$ – Snowbody Jun 18 '15 at 17:34
  • 1
    \$\begingroup\$ Any() works much the same as FirstOrDefault(), but rather than returning the first item, it returns true, and rather than returning null, it returns false. I would expect their performance to be effectively identical for in-memory operations. There may be more of a difference in scenarios where the query is translated to SQL or otherwise not run in-memory on .NET objects, but any differences will be very situational. \$\endgroup\$ – Taudris Jun 18 '15 at 18:26
13
\$\begingroup\$

What you are doing is filtering PersonResultList to only include female genders. Joining to add information from PersonList expresses this more clearly and efficiently.

Using extension methods:

PersonResultList = PersonResultList.Join(
    PersonList.Where(_ => _.Gender == "female"), 
    personResult => personResult.PersonId,
    person => person.PersonId, 
    (personResult, person) => personResult);

Pretty much the same thing, but clearer:

PersonResultList = from personResult in PersonResultList
                   join person in PersonList on personResult.PersonId equals person.PersonId
                   where person.Gender == "female"
                   select personResult;
\$\endgroup\$
  • \$\begingroup\$ I do prefer extension method for LINQ so I did not thought of this approach. I'll keep that in mind next time I'm writng LINQ queries. \$\endgroup\$ – WizLiz Jun 18 '15 at 12:57
  • \$\begingroup\$ My first code block actually is the extension method approach you prefer. It's more efficient than Heslacher's answer (which is manually joining via nested loops), and also speaks the intent of the code more clearly than manually implementing nested loops. Either block here is much better LINQ. \$\endgroup\$ – jnm2 Jun 18 '15 at 13:53
  • 5
    \$\begingroup\$ ++ sometimes the query syntax is much more understandable. \$\endgroup\$ – RubberDuck Jun 18 '15 at 22:32
  • \$\begingroup\$ @RubberDuck I agree with you in certain cases. However, it gets cluttered fast if you need to do a series of operations (eg group, sort, skip, select, join, select), at least IMO \$\endgroup\$ – Basic Jun 19 '15 at 7:10
  • \$\begingroup\$ Agreed @Basic. I did say sometimes after all. =;)- \$\endgroup\$ – RubberDuck Jun 19 '15 at 10:43

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.