Efficiency of creating delegate instance inline of LINQ query?

Question

Following are two examples that do the same thing in different ways. I'm comparing them.

Version 1

For the sake of an example, define any method to create and return an ExpandoObject from an XElement based on business logic:

var ToExpando = new Func<XElement, ExpandoObject>(xClient =>
{
    dynamic o = new ExpandoObject();    
    o.OnlineDetails = new ExpandoObject();
    o.OnlineDetails.Password = xClient.Element(XKey.onlineDetails).Element(XKey.password).Value;
    o.OnlineDetails.Roles = xClient.Element(XKey.onlineDetails).Element(XKey.roles).Elements(XKey.roleId).Select(xroleid => xroleid.Value);
    // More fields TBD.
}

Call the above delegate from a LINQ to XML query:

var qClients =
    from client in xdoc.Root.Element(XKey.clients).Elements(XKey.client)
    select ToExpando(client);

---

Version 2

Do it all in the LINQ query, including creation and call to Func delegate.

var qClients =
from client in xdoc.Root.Element(XKey.clients).Elements(XKey.client)
select (new Func<ExpandoObject>(() =>
        {
            dynamic o = new ExpandoObject();
            o.OnlineDetails = new ExpandoObject();
            o.OnlineDetails.Password = client.Element(XKey.onlineDetails).Element(XKey.password).Value;
            o.OnlineDetails.Roles = client.Element(XKey.onlineDetails).Element(XKey.roles).Elements(XKey.roleId).Select(xroleid => xroleid.Value);
            // More fields TBD.
  return o;
  }))();

---

Considering delegate creation is in the select part, is Version 2 inefficient? Is it managed or optimized by either the C# compiler or runtime so it won't matter?

I like Version 2 for its tightness (keeping the object creation logic in the query), but am aware it might not be viable depending on what the compiler or runtime does.

Answer 1

The latter approach looks pretty horrible to me. I believe it will have to genuinely create a new delegate each time as you're capturing a different client each time, but personally I wouldn't do it that way at all. Given that you've got real statements in there, why not write a normal method?

private static ToExpando(XElement client)
{
    // Possibly use an object initializer instead?
    dynamic o = new ExpandoObject();    
    o.OnlineDetails = new ExpandoObject();
    o.OnlineDetails.Password = client.Element(XKey.onlineDetails)
                                     .Element(XKey.password).Value;
    o.OnlineDetails.Roles = client.Element(XKey.onlineDetails)
                                  .Element(XKey.roles)
                                  .Elements(XKey.roleId)
                                  .Select(xroleid => xroleid.Value);
    return o;
}

and then query it with:

var qClients = xdoc.Root.Element(XKey.clients)
                        .Elements(XKey.client)
                        .Select(ToExpando);

I would be much more concerned about the readability of the code than the performance of creating delegates, which is generally pretty quick. I don't think there's any need to use nearly as many lambdas as you currently seem keen to do. Think about when you come back to this code in a year's time. Are you really going to find the nested lambda easier to understand than a method?

(By the way, separating the conversion logic into a method makes that easy to test in isolation...)

EDIT: Even if you do want to do it all in the LINQ expression, why are you so keen to create another level of indirection? Just because query expressions don't allow statement lambdas? Given that you're doing nothing but a simple select, that's easy enough to cope with:

var qClients = xdoc.Root
           .Element(XKey.clients)
           .Elements(XKey.client)
           .Select(client => {
               dynamic o = new ExpandoObject();
               o.OnlineDetails = new ExpandoObject();
               o.OnlineDetails.Password = client.Element(XKey.onlineDetails)
                                                .Element(XKey.password).Value;
               o.OnlineDetails.Roles = client.Element(XKey.onlineDetails)
                                             .Element(XKey.roles)
                                             .Elements(XKey.roleId)
                                             .Select(xroleid => xroleid.Value);
               return o; 
           });

Answer 2

It is true that your second version creates new Func instance repeatedly - however, this just means allocating some small object (closure) and using pointer to a function. I don't think this is a large overhead compared to dynamic lookups that you need to perform in the body of the delegate (to work with dynamic objects).

Alternatively, you could declare a local lambda function like this:

Func<XElement, ExpandoObject> convert = client => {
    dynamic o = new ExpandoObject();
    o.OnlineDetails = new ExpandoObject();
    o.OnlineDetails.Password = 
       client.Element(XKey.onlineDetails).Element(XKey.password).Value;
    o.OnlineDetails.Roles = client.Element(XKey.onlineDetails).
       Element(XKey.roles).Elements(XKey.roleId).
       Select(xroleid => xroleid.Value);
    // More fields TBD.
    return o;
}

var qClients =
    from client in xdoc.Root.Element(XKey.clients).Elements(XKey.client)
    select convert(client);

This way, you can create just a single delegate, but keep the code that does the conversion close to the code that implements the query.

Another option would be to use anonymous types instead - what are the reasons for using ExpandoObject in your scenario? The only limitation of anonymous types would be that you may not be able to access them from other assemblies (they are internal), but working with them using dynamic should be fine...

Your select could look like:

select new { OnlineDetails = new { Password = ..., Roles = ... }}

Finally, you could also use Reflection to convert anonymous type to ExpandoObject, but that would probably be even more inefficient (i.e. very difficult to write efficiently)