4
\$\begingroup\$

I have a "GetPersonsPerDepartment()" method returning a IQueryable<IEnumerable<Person>> while I'm trying to get a simple List<Person> or IEnumerable<Person> with all the persons from every department. I wrote a simple method GetAllPersons() that solves this with a loop but I feel I must be missing a simple command to get the same result:

private List<Person> GetAllPersons()
{
    List<Person> allPersons = new List<Person>();

    foreach(var Persons in GetPersonsPerDepartment())
    {
        allPersons.AddRange(Persons);
    }

    return allPersons;
}

private IQueryable<IEnumerable<Person>> GetPersonsPerDepartment()
{
    IQueryable<IEnumerable<Person>> Persons;

    using (var context = MySource.GetPersonsContext())
    {
        var library = "Personlist";
        var PersonFolderList = context.Web.Lists.GetByTitle(library);
        context.Load<List>(PersonFolderList);

        var PersonFolderListItems = PersonFolderList.GetItems(new CamlQuery());
        context.Load<ListItemCollection>(PersonFolderListItems);

        context.ExecuteQuery();

        Persons = PersonFolderListItems.Where(x => x.FileSystemObjectType == FileSystemObjectType.Folder).Select(x => GetPersonsFromFolderListItem(x, context, PersonFolderList));
    }

    return Persons;
}
\$\endgroup\$
22
\$\begingroup\$

Use SelectMany. As in, allPersons.SelectMany(s => s);. This flattens the sequence for you.

\$\endgroup\$
  • 11
    \$\begingroup\$ Not only one-liner code but also a one-liner review - there should be a badge for it ;-] \$\endgroup\$ – t3chb0t Jan 10 at 13:28
  • 4
    \$\begingroup\$ There seems to be :). The +10 answer badge. \$\endgroup\$ – Hosch250 Jan 10 at 15:48
  • 1
    \$\begingroup\$ While correct, I find SelectMany(s => s) does not really convey the "concatenate all of these together" meaning very well. If you're not a heavy LINQ user, it's going to confuse you initially. (I've had coworkers be confused by it.) One might consider wrapping this inside a method: public static ConcatAll<T>(this IEnumerable<IEnumerable<T>> e) { return e.SelectMany(i = i); }. \$\endgroup\$ – jpmc26 Jan 11 at 2:03
  • 1
    \$\begingroup\$ @jpmc26 I've had coworkers be confused by it - then they either should learn how to use LINQ correctly or leave writing software to people who can understand something as simple as SelectMany.. Creating extensions like ConcatAll is even more confusing. \$\endgroup\$ – t3chb0t Jan 12 at 14:30
  • \$\begingroup\$ @t3chb0t Giving things appropriate, straightforward names that clearly communicates their behavior increases confusion? Wat. The best kind of code is code that can be understood at a glance. There is absolutely nothing wrong with being clear enough that other people understand something immediately. Honestly, we'd all be better off if the .NET standard library was written with that mindset. How did you get 34k rep on a code review site thinking like that? \$\endgroup\$ – jpmc26 Jan 12 at 17:54
3
\$\begingroup\$

Is this a bug or dirty workaround?

There is one thing in your code that sooner or later will bite you and I don't quite understnd why it didn't...

You're returing an IQueryable<IEnumerable<Person>> after you have disposed your context. I'm not sure what workarounds your are using there because you provided very little context but your code actually shouldn't work.

I suppose it's Entity Framework and with it you should never return IQuerybale if you're disposing its context by the same method but always materialize the query before returnig the result, e.g. with ToList etc.

\$\endgroup\$
  • \$\begingroup\$ Returning an IQueryable is a perfectly valid strategy with EF--it won't actually do the query until you materialize it, and it allows other parts of the code to add additional filters/tranformations to the data that will be executed on the DB, making it more performant. Of course, this only works in certain cases where you get the context when you need it, instead of being in a using block (like if you are working from an IGenericRepository type thing). \$\endgroup\$ – Hosch250 Jan 10 at 19:29
  • 2
    \$\begingroup\$ @Hosch250 I know how it works and clarified what I meant ;-] OP does have a using block here thus it's super confusing why it actually works... it shouldn't and IMO there is some dirty workaround becuase if it's materialized then it shouldn't be an IQueryable and if it's not then the context should not be disposed. \$\endgroup\$ – t3chb0t Jan 10 at 19:34

Your Answer

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

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