0
\$\begingroup\$

I have a simple projection in my code. Taking away the majority of the fields which are not of interest, it looks like the following:

var result = employments.Select(x => new EmpListItem
        {
            EndDate = x.EmpRelationship.OrderByDescending(y => y.EndTime).FirstOrDefault().CancelledBeforeTime ? 
                        x.EmpRelationship.OrderByDescending(y => y.EndTime).FirstOrDefault().CancelEndTime : 
                        x.EmpRelationship.OrderByDescending(y => y.EndTime).FirstOrDefault().EndTime,
            JobType = x.JobType
        });

I really dislike how I use x.EmpRelationship.OrderByDescending(y => y.EndTime).FirstOrDefault() three times. It looks ugly and readability is poor.

Is there a way to use this only once while staying inline? By inline I mean that I don't want to write any code outside of the projection.

\$\endgroup\$
5
\$\begingroup\$

You can extract out that repeat call to a variable inside your Select.

var result = employments.Select(x => 
{
    var empRelationship = x.EmpRelationship.OrderByDescending(y => y.EndTime).FirstOrDefault();

    return new EmpListItem
    {
        EndDate = empRelationship.CancelledBeforeTime ? 
                      empRelationship.CancelEndTime : 
                      empRelationship.EndTime,
        JobType = x.JobType
    };
});

This will also speed up execution.

The trick behind this is remembering you can execute more than just one statement inside a lambda by using { and }

\$\endgroup\$
4
  • \$\begingroup\$ I had to write employments.AsEnumerable.Select.... since employments is of type IQueryable (which I should've mentioned in the question). When I did that your solution works perfectly! Do you know if first converting to IEnumerable and then to IQueryable is a heavy on the performance side? I need to return an IQueryable but as mentioned above it doesn't allow me to write your solution out if employment is not of type IEnumerable \$\endgroup\$
    – Force444
    May 19 '15 at 10:12
  • \$\begingroup\$ I don't know off-hand how the conversion between IEnumerable and IQueryable is done, my recommendation is to only worry about it if that section of code becomes a performance bottleneck. Premature optimisation is a sure-fire way to waste your time and ruin your code. \$\endgroup\$
    – Nick Udell
    May 19 '15 at 11:28
  • \$\begingroup\$ Cheers:) no performance issues, so I'll leave it as it is. \$\endgroup\$
    – Force444
    May 19 '15 at 11:39
  • 1
    \$\begingroup\$ I would also point out that First() might be a better choice than FirstOrDefault(), since there is no null check before accessing properties on empRelationship. \$\endgroup\$
    – Matt Cole
    May 20 '15 at 19:50

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.