Refactor Linq query

Question

I am trying to cut this linq down

 var sys = db.tlkpSystems
 .Where(a => db.tlkpSettings.Where(e => e.Hidden < 3)
 .Select(o => o.System)
 .ToList().Contains(a.System))      //cannot get this part in?
 .OrderBy(a => a.SystemName).ToList();

foreach (var item in sys)
    model.Add(new SettingSystem { 
        System = item.System, 
        SystemName = item.SystemName 
});

I have tried the following:

   List<SettingSystem> model = new List<SettingSystem>();
   model = db.tlkpSettings.Where(e => e.Hidden < 3)
     .OrderBy(e => e.Setting)
     .Select(e => new SettingSystem
     {
      System = e.System,
      SystemName = e.Setting
      }).ToList();

How can I call the .Contains(a.System) part in my query?

Thanks

@Mafii any reason why .ToList() should not be used? I am new to linq — Harry, Mar 23 '16 at 15:57
Yes. http://stackoverflow.com/questions/15516462/is-there-a-performance-impact-when-calling-tolist — uTeisT, Mar 23 '16 at 16:03
Problem is I cannot figure out how to do this part in my version of linq .ToList().Contains(a.System)) //cannot get this part in? — Harry, Mar 23 '16 at 16:08
@MelanciaUK Please read [a guide to CR for SO users](http://meta.codereview.stackexchange.com/q/5777/23788) before making further recommendations to sites you don't even have a profile for. — Mathieu Guindon, Mar 23 '16 at 16:09
@Mafii Code Review deals with code that works as intended. This code clearly doesn't work as intended. Please refrain from recommending sites you don't have a user profile for and/or aren't familiar with. — Mathieu Guindon, Mar 23 '16 at 16:13

score 3 · Accepted Answer · answered Mar 23 '16 at 16:17

3

Some general rules when working with LINQ to Entities:

Avoid using ToList inside the query. It prevents EF to build a correct SQL query.
Don't use Contains when working with entities (tables). Use Any or joins.

Here is your query (in case System is not an entity navigation property):

var sys = db.tlkpSystems
    .Where(a => db.tlkpSettings.Any(e => e.Hidden < 3 && e.System == a.System))
    .OrderBy(a => a.SystemName).ToList();

answered Mar 23 '16 at 16:17

Ivan Stoev

195,425
15
312
343

Seems like the `ToList()` isn't needed since all he does is iterate. – Ed T Mar 23 '16 at 16:44
to list is needed as I return a list – Harry Mar 23 '16 at 16:46
@EdT Correct, the above is just to present the solution of the problematic part of the OP original query. Instead of `sys` variable and `ToList` \, he can add `.Select(...).ToList()` and get directly the result as in the second query. – Ivan Stoev Mar 23 '16 at 16:52

score 1 · Answer 2 · answered Mar 23 '16 at 16:21

As an addendum, there is also AsEnumerable for when you must pull a query into memory (such as calling methods within another clause). This is generally better than ToList or ToArray since it'll enumerate the query, rather than enumerating, putting together a List/Array, and then enumerating that collection.

Refactor Linq query

2 Answers2